system/serenity
system/serenity
1
0
Fork 0
mirror of https://github.com/SerenityOS/serenity synced 2024-10-06 16:09:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

LibJS: Don't evaluate computed MemberExpression LHS twice in assignments

Browse source 
The following snippet would cause "i" to be incremented twice(!):

    let a = []
    let i = 0
    a[++i] += 0

This patch solves the issue by remembering the base object and property
name for computed MemberExpression LHS in codegen. We the store the
result of the assignment to the same object and property (instead of
computing the LHS again).

3 new passes on test262. :^)
This commit is contained in:
Andreas Kling 2023-10-04 15:22:07 +02:00
parent 2083376618
commit 732b39d120
3 changed files with 42 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
View file

@ -525,7 +525,7 @@ Bytecode::CodeGenerationErrorOr<void> AssignmentExpression::generate_bytecode(By
VERIFY(m_lhs.has<NonnullRefPtr<Expression const>>());
auto& lhs = m_lhs.get<NonnullRefPtr<Expression const>>();
TRY(generator.emit_load_from_reference(lhs));
auto reference_registers = TRY(generator.emit_load_from_reference(lhs));
Bytecode::BasicBlock* rhs_block_ptr { nullptr };
Bytecode::BasicBlock* end_block_ptr { nullptr };
@ -615,7 +615,10 @@ Bytecode::CodeGenerationErrorOr<void> AssignmentExpression::generate_bytecode(By
};
}
TRY(generator.emit_store_to_reference(lhs));
if (reference_registers.has_value())
TRY(generator.emit_store_to_reference(*reference_registers));
else
TRY(generator.emit_store_to_reference(lhs));
if (end_block_ptr) {
generator.emit<Bytecode::Op::Jump>(Bytecode::Label { *end_block_ptr });
@ -1049,7 +1052,8 @@ Bytecode::CodeGenerationErrorOr<void> ArrayExpression::generate_bytecode(Bytecod
Bytecode::CodeGenerationErrorOr<void> MemberExpression::generate_bytecode(Bytecode::Generator& generator) const
{
Bytecode::Generator::SourceLocationScope scope(generator, *this);
return generator.emit_load_from_reference(*this);
(void)TRY(generator.emit_load_from_reference(*this));
return {};
}
Bytecode::CodeGenerationErrorOr<void> FunctionDeclaration::generate_bytecode(Bytecode::Generator& generator) const
@ -2249,7 +2253,7 @@ Bytecode::CodeGenerationErrorOr<void> TaggedTemplateLiteral::generate_bytecode(B
Bytecode::CodeGenerationErrorOr<void> UpdateExpression::generate_bytecode(Bytecode::Generator& generator) const
{
Bytecode::Generator::SourceLocationScope scope(generator, *this);
TRY(generator.emit_load_from_reference(*m_argument));
auto reference_registers = TRY(generator.emit_load_from_reference(*m_argument));
Optional<Bytecode::Register> previous_value_for_postfix_reg;
if (!m_prefixed) {
@ -2263,7 +2267,10 @@ Bytecode::CodeGenerationErrorOr<void> UpdateExpression::generate_bytecode(Byteco
else
generator.emit<Bytecode::Op::Decrement>();
TRY(generator.emit_store_to_reference(*m_argument));
if (reference_registers.has_value())
TRY(generator.emit_store_to_reference(*reference_registers));
else
TRY(generator.emit_store_to_reference(*m_argument));
if (!m_prefixed)
generator.emit<Bytecode::Op::Load>(*previous_value_for_postfix_reg);

26
Userland/Libraries/LibJS/Bytecode/Generator.cpp
View file

@ -189,12 +189,12 @@ CodeGenerationErrorOr<Generator::ReferenceRegisters> Generator::emit_super_refer
};
}
CodeGenerationErrorOr<void> Generator::emit_load_from_reference(JS::ASTNode const& node)
CodeGenerationErrorOr<Optional<Generator::ReferenceRegisters>> Generator::emit_load_from_reference(JS::ASTNode const& node)
{
if (is<Identifier>(node)) {
auto& identifier = static_cast<Identifier const&>(node);
TRY(identifier.generate_bytecode(*this));
return {};
return Optional<ReferenceRegisters> {};
}
if (is<MemberExpression>(node)) {
auto& expression = static_cast<MemberExpression const&>(node);
@ -213,15 +213,24 @@ CodeGenerationErrorOr<void> Generator::emit_load_from_reference(JS::ASTNode cons
auto identifier_table_ref = intern_identifier(verify_cast<Identifier>(expression.property()).string());
emit_get_by_id_with_this(identifier_table_ref, super_reference.this_value);
}
return super_reference;
} else {
TRY(expression.object().generate_bytecode(*this));
if (expression.is_computed()) {
auto object_reg = allocate_register();
emit<Bytecode::Op::Store>(object_reg);
TRY(expression.property().generate_bytecode(*this));
auto property_reg = allocate_register();
emit<Bytecode::Op::Store>(property_reg);
emit<Bytecode::Op::GetByValue>(object_reg);
return ReferenceRegisters {
.base = object_reg,
.referenced_name = property_reg,
.this_value = object_reg,
};
} else if (expression.property().is_identifier()) {
auto identifier_table_ref = intern_identifier(verify_cast<Identifier>(expression.property()).string());
emit_get_by_id(identifier_table_ref);
@ -235,7 +244,7 @@ CodeGenerationErrorOr<void> Generator::emit_load_from_reference(JS::ASTNode cons
};
}
}
return {};
return Optional<ReferenceRegisters> {};
}
VERIFY_NOT_REACHED();
}
@ -306,6 +315,15 @@ CodeGenerationErrorOr<void> Generator::emit_store_to_reference(JS::ASTNode const
};
}
CodeGenerationErrorOr<void> Generator::emit_store_to_reference(ReferenceRegisters const& reference_registers)
{
if (reference_registers.base == reference_registers.this_value)
emit<Bytecode::Op::PutByValue>(reference_registers.base, reference_registers.referenced_name.value());
else
emit<Bytecode::Op::PutByValueWithThis>(reference_registers.base, reference_registers.referenced_name.value(), reference_registers.this_value);
return {};
}
CodeGenerationErrorOr<void> Generator::emit_delete_reference(JS::ASTNode const& node)
{
if (is<Identifier>(node)) {

14
Userland/Libraries/LibJS/Bytecode/Generator.h
View file

@ -74,15 +74,17 @@ public:
op->set_source_record({ m_current_ast_node->start_offset(), m_current_ast_node->end_offset() });
}
CodeGenerationErrorOr<void> emit_load_from_reference(JS::ASTNode const&);
struct ReferenceRegisters {
Register base; // [[Base]]
Optional<Register> referenced_name; // [[ReferencedName]]
Register this_value; // [[ThisValue]]
};
CodeGenerationErrorOr<Optional<ReferenceRegisters>> emit_load_from_reference(JS::ASTNode const&);
CodeGenerationErrorOr<void> emit_store_to_reference(JS::ASTNode const&);
CodeGenerationErrorOr<void> emit_store_to_reference(ReferenceRegisters const&);
CodeGenerationErrorOr<void> emit_delete_reference(JS::ASTNode const&);
struct ReferenceRegisters {
Register base; // [[Base]]
Optional<Bytecode::Register> referenced_name; // [[ReferencedName]]
Register this_value; // [[ThisValue]]
};
CodeGenerationErrorOr<ReferenceRegisters> emit_super_reference(MemberExpression const&);
void emit_set_variable(JS::Identifier const& identifier, Bytecode::Op::SetVariable::InitializationMode initialization_mode = Bytecode::Op::SetVariable::InitializationMode::Set, Bytecode::Op::EnvironmentMode mode = Bytecode::Op::EnvironmentMode::Lexical);