[Interpreter]Elide redundant load context bytecode

We found there are redundant load context operations in
some bytecode array.

like this:
LdaImmutableCurrentContextSlot [1]
Star0
......   (don’t edit accumulator)
LdaImmutableCurrentContextSlot [1]
Star1
Add r1

In that case, we could modify this bytecode array as:
LdaImmutableCurrentContextSlot [1]
Star0
......   (don’t edit accumulator)
Add r0

This CL will elide these redundant bytecodes
(LdaImmutableCurrentContextSlot and Star1), because there is no
side effect for loading context, and this context slot is immutable.

Change-Id: Ia26f4b934d3bd1d48c50c0c4699ba7942939991c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3816221
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82641}

src/interpreter/bytecode-array-builder.h

@@ -490,6 +490,10 @@ class V8_EXPORT_PRIVATE BytecodeArrayBuilder final {
   // constant pool.
   BytecodeJumpTable* AllocateJumpTable(int size, int case_value_base);
 
+  BytecodeRegisterOptimizer* GetRegisterOptimizer() {
+    return register_optimizer_;
+  }
+
   // Gets a constant pool entry.
   size_t GetConstantPoolEntry(const AstRawString* raw_string);
   size_t GetConstantPoolEntry(AstBigInt bigint);

src/interpreter/bytecode-generator.cc

@@ -23,6 +23,7 @@
 #include "src/interpreter/bytecode-jump-table.h"
 #include "src/interpreter/bytecode-label.h"
 #include "src/interpreter/bytecode-register-allocator.h"
+#include "src/interpreter/bytecode-register-optimizer.h"
 #include "src/interpreter/bytecode-register.h"
 #include "src/interpreter/control-flow-builders.h"
 #include "src/logging/local-logger.h"
@@ -3534,6 +3535,21 @@ void BytecodeGenerator::VisitVariableProxy(VariableProxy* proxy) {
   BuildVariableLoad(proxy->var(), proxy->hole_check_mode());
 }
 
+bool BytecodeGenerator::IsVariableInRegister(Variable* var, Register reg) {
+  BytecodeRegisterOptimizer* optimizer = builder()->GetRegisterOptimizer();
+  if (optimizer) {
+    return optimizer->IsVariableInRegister(var, reg);
+  }
+  return false;
+}
+
+void BytecodeGenerator::SetVariableInRegister(Variable* var, Register reg) {
+  BytecodeRegisterOptimizer* optimizer = builder()->GetRegisterOptimizer();
+  if (optimizer) {
+    optimizer->SetVariableInRegister(var, reg);
+  }
+}
+
 void BytecodeGenerator::BuildVariableLoad(Variable* variable,
                                           HoleCheckMode hole_check_mode,
                                           TypeofMode typeof_mode) {
@@ -3593,12 +3609,20 @@ void BytecodeGenerator::BuildVariableLoad(Variable* variable,
           (variable->maybe_assigned() == kNotAssigned)
               ? BytecodeArrayBuilder::kImmutableSlot
               : BytecodeArrayBuilder::kMutableSlot;
+      Register acc = Register::virtual_accumulator();
+      if (immutable == BytecodeArrayBuilder::kImmutableSlot &&
+          IsVariableInRegister(variable, acc)) {
+        return;
+      }
 
       builder()->LoadContextSlot(context_reg, variable->index(), depth,
                                  immutable);
       if (hole_check_mode == HoleCheckMode::kRequired) {
         BuildThrowIfHole(variable);
       }
+      if (immutable == BytecodeArrayBuilder::kImmutableSlot) {
+        SetVariableInRegister(variable, acc);
+      }
       break;
     }
     case VariableLocation::LOOKUP: {

src/interpreter/bytecode-generator.h

@@ -253,6 +253,10 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
                              const AstRawString* name);
   void BuildStoreGlobal(Variable* variable);
 
+  bool IsVariableInRegister(Variable* var, Register reg);
+
+  void SetVariableInRegister(Variable* var, Register reg);
+
   void BuildVariableLoad(Variable* variable, HoleCheckMode hole_check_mode,
                          TypeofMode typeof_mode = TypeofMode::kNotInside);
   void BuildVariableLoadForAccumulatorValue(

src/interpreter/bytecode-register-optimizer.cc

@@ -22,13 +22,15 @@ class BytecodeRegisterOptimizer::RegisterInfo final : public ZoneObject {
         materialized_(materialized),
         allocated_(allocated),
         needs_flush_(false),
+        var_in_reg_(nullptr),
         next_(this),
         prev_(this) {}
   RegisterInfo(const RegisterInfo&) = delete;
   RegisterInfo& operator=(const RegisterInfo&) = delete;
 
   void AddToEquivalenceSetOf(RegisterInfo* info);
-  void MoveToNewEquivalenceSet(uint32_t equivalence_id, bool materialized);
+  void MoveToNewEquivalenceSet(uint32_t equivalence_id, bool materialized,
+                               Variable* var = nullptr);
   bool IsOnlyMemberOfEquivalenceSet() const;
   bool IsOnlyMaterializedMemberOfEquivalenceSet() const;
   bool IsInSameEquivalenceSet(RegisterInfo* info) const;
@@ -76,6 +78,8 @@ class BytecodeRegisterOptimizer::RegisterInfo final : public ZoneObject {
   // Indicates if a register should be processed when calling Flush().
   bool needs_flush() const { return needs_flush_; }
   void set_needs_flush(bool needs_flush) { needs_flush_ = needs_flush; }
+  Variable* var_in_reg() const { return var_in_reg_; }
+  void set_var_in_reg(Variable* var) { var_in_reg_ = var; }
 
  private:
   Register register_;
@@ -83,6 +87,7 @@ class BytecodeRegisterOptimizer::RegisterInfo final : public ZoneObject {
   bool materialized_;
   bool allocated_;
   bool needs_flush_;
+  Variable* var_in_reg_;
 
   // Equivalence set pointers.
   RegisterInfo* next_;
@@ -102,15 +107,17 @@ void BytecodeRegisterOptimizer::RegisterInfo::AddToEquivalenceSetOf(
   next_->prev_ = this;
   set_equivalence_id(info->equivalence_id());
   set_materialized(false);
+  set_var_in_reg(info->var_in_reg());
 }
 
 void BytecodeRegisterOptimizer::RegisterInfo::MoveToNewEquivalenceSet(
-    uint32_t equivalence_id, bool materialized) {
+    uint32_t equivalence_id, bool materialized, Variable* var) {
   next_->prev_ = prev_;
   prev_->next_ = next_;
   next_ = prev_ = this;
   equivalence_id_ = equivalence_id;
   materialized_ = materialized;
+  var_in_reg_ = var;
 }
 
 bool BytecodeRegisterOptimizer::RegisterInfo::IsOnlyMemberOfEquivalenceSet()
@@ -118,6 +125,25 @@ bool BytecodeRegisterOptimizer::RegisterInfo::IsOnlyMemberOfEquivalenceSet()
   return this->next_ == this;
 }
 
+void BytecodeRegisterOptimizer::SetVariableInRegister(Variable* var,
+                                                      Register reg) {
+  RegisterInfo* info = GetRegisterInfo(reg);
+  PushToRegistersNeedingFlush(info);
+  info->set_var_in_reg(var);
+}
+
+Variable* BytecodeRegisterOptimizer::GetVariableInRegister(Register reg) {
+  RegisterInfo* info = GetRegisterInfo(reg);
+  return info->var_in_reg();
+}
+
+bool BytecodeRegisterOptimizer::IsVariableInRegister(Variable* var,
+                                                     Register reg) {
+  DCHECK_NOT_NULL(var);
+  RegisterInfo* info = GetRegisterInfo(reg);
+  return info->var_in_reg() == var;
+}
+
 bool BytecodeRegisterOptimizer::RegisterInfo::
     IsOnlyMaterializedMemberOfEquivalenceSet() const {
   DCHECK(materialized());
@@ -252,6 +278,10 @@ BytecodeRegisterOptimizer::BytecodeRegisterOptimizer(
 }
 
 void BytecodeRegisterOptimizer::PushToRegistersNeedingFlush(RegisterInfo* reg) {
+  // Flushing is required in two cases:
+  // 1) Two or more registers in the same equivalence set.
+  // 2) Binding a variable to a register.
+  flush_required_ = true;
   if (!reg->needs_flush()) {
     reg->set_needs_flush(true);
     registers_needing_flushed_.push_back(reg);
@@ -280,6 +310,7 @@ void BytecodeRegisterOptimizer::Flush() {
   for (RegisterInfo* reg_info : registers_needing_flushed_) {
     if (!reg_info->needs_flush()) continue;
     reg_info->set_needs_flush(false);
+    reg_info->set_var_in_reg(nullptr);
 
     RegisterInfo* materialized = reg_info->materialized()
                                      ? reg_info
@@ -372,9 +403,6 @@ void BytecodeRegisterOptimizer::AddToEquivalenceSet(
   // Equivalence class is now of size >= 2, so we make sure it will be flushed.
   PushToRegistersNeedingFlush(non_set_member);
   non_set_member->AddToEquivalenceSetOf(set_member);
-  // Flushing is only required when two or more registers are placed
-  // in the same equivalence set.
-  flush_required_ = true;
 }
 
 void BytecodeRegisterOptimizer::RegisterTransfer(RegisterInfo* input_info,

src/interpreter/bytecode-register-optimizer.h

@@ -5,6 +5,7 @@
 #ifndef V8_INTERPRETER_BYTECODE_REGISTER_OPTIMIZER_H_
 #define V8_INTERPRETER_BYTECODE_REGISTER_OPTIMIZER_H_
 
+#include "src/ast/variables.h"
 #include "src/base/compiler-specific.h"
 #include "src/common/globals.h"
 #include "src/interpreter/bytecode-register-allocator.h"
@@ -111,6 +112,15 @@ class V8_EXPORT_PRIVATE BytecodeRegisterOptimizer final
   // operand.
   RegisterList GetInputRegisterList(RegisterList reg_list);
 
+  // Maintain the map between Variable and Register.
+  void SetVariableInRegister(Variable* var, Register reg);
+
+  // Get the variable in the reg.
+  Variable* GetVariableInRegister(Register reg);
+
+  // Return true if the var is in the reg.
+  bool IsVariableInRegister(Variable* var, Register reg);
+
   int maxiumum_register_index() const { return max_register_index_; }
 
  private:

test/unittests/interpreter/bytecode-generator-unittest.cc

@@ -3186,6 +3186,24 @@ TEST_F(BytecodeGeneratorTest, TemplateLiterals) {
                      LoadGolden("TemplateLiterals.golden")));
 }
 
+TEST_F(BytecodeGeneratorTest, ElideRedundantLoadOperationOfImmutableContext) {
+  printer().set_wrap(false);
+  printer().set_test_function_name("test");
+
+  std::string snippets[] = {
+      "var test;\n"
+      "(function () {\n"
+      "  var a = {b: 2, c: 3};\n"
+      "  function foo() {a.b = a.c;}\n"
+      "  foo();\n"
+      "  test = foo;\n"
+      "})();\n"};
+
+  CHECK(CompareTexts(
+      BuildActual(printer(), snippets),
+      LoadGolden("ElideRedundantLoadOperationOfImmutableContext.golden")));
+}
+
 }  // namespace interpreter
 }  // namespace internal
 }  // namespace v8

test/unittests/interpreter/bytecode_expectations/ElideRedundantLoadOperationOfImmutableContext.golden

@@ -0,0 +1,36 @@
+#
+# Autogenerated by generate-bytecode-expectations.
+#
+
+---
+wrap: no
+test function name: test
+
+---
+snippet: "
+  var test;
+  (function () {
+    var a = {b: 2, c: 3};
+    function foo() {a.b = a.c;}
+    foo();
+    test = foo;
+  })();
+"
+frame size: 1
+parameter count: 1
+bytecode array length: 13
+bytecodes: [
+  /*   67 S> */ B(LdaImmutableCurrentContextSlot), U8(2),
+                B(Star0),
+  /*   75 E> */ B(GetNamedProperty), R(0), U8(0), U8(0),
+  /*   71 E> */ B(SetNamedProperty), R(0), U8(1), U8(2),
+                B(LdaUndefined),
+  /*   77 S> */ B(Return),
+]
+constant pool: [
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["c"],
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["b"],
+]
+handlers: [
+]
+