From c4a0e4a10f3b10b247f811a47ce2bf922107fd30 Mon Sep 17 00:00:00 2001 From: "wenqin.yang" Date: Mon, 22 Aug 2022 10:05:51 +0800 Subject: [PATCH] [Interpreter]Elide redundant load context bytecode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Commit-Queue: Shu-yu Guo Cr-Commit-Position: refs/heads/main@{#82641} --- src/interpreter/bytecode-array-builder.h | 4 ++ src/interpreter/bytecode-generator.cc | 24 ++++++++++++ src/interpreter/bytecode-generator.h | 4 ++ .../bytecode-register-optimizer.cc | 38 ++++++++++++++++--- src/interpreter/bytecode-register-optimizer.h | 10 +++++ .../bytecode-generator-unittest.cc | 18 +++++++++ ...dantLoadOperationOfImmutableContext.golden | 36 ++++++++++++++++++ 7 files changed, 129 insertions(+), 5 deletions(-) create mode 100644 test/unittests/interpreter/bytecode_expectations/ElideRedundantLoadOperationOfImmutableContext.golden diff --git a/src/interpreter/bytecode-array-builder.h b/src/interpreter/bytecode-array-builder.h index ececc53a2f..bf0c712adb 100644 --- a/src/interpreter/bytecode-array-builder.h +++ b/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); diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc index 09210371e2..f99c02e8ac 100644 --- a/src/interpreter/bytecode-generator.cc +++ b/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: { diff --git a/src/interpreter/bytecode-generator.h b/src/interpreter/bytecode-generator.h index c3883de2a8..e310eb3f7f 100644 --- a/src/interpreter/bytecode-generator.h +++ b/src/interpreter/bytecode-generator.h @@ -253,6 +253,10 @@ class BytecodeGenerator final : public AstVisitor { 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( diff --git a/src/interpreter/bytecode-register-optimizer.cc b/src/interpreter/bytecode-register-optimizer.cc index 82b0a82552..a733b5bab4 100644 --- a/src/interpreter/bytecode-register-optimizer.cc +++ b/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, diff --git a/src/interpreter/bytecode-register-optimizer.h b/src/interpreter/bytecode-register-optimizer.h index 378e345fa8..52123df198 100644 --- a/src/interpreter/bytecode-register-optimizer.h +++ b/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: diff --git a/test/unittests/interpreter/bytecode-generator-unittest.cc b/test/unittests/interpreter/bytecode-generator-unittest.cc index 4a73bf5ad1..a0cf4552b6 100644 --- a/test/unittests/interpreter/bytecode-generator-unittest.cc +++ b/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 diff --git a/test/unittests/interpreter/bytecode_expectations/ElideRedundantLoadOperationOfImmutableContext.golden b/test/unittests/interpreter/bytecode_expectations/ElideRedundantLoadOperationOfImmutableContext.golden new file mode 100644 index 0000000000..bfb6c09785 --- /dev/null +++ b/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: [ +] +