From 687362831717e9a63fabc8a35211296b873db576 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 8 May 2024 12:43:08 +0200 Subject: [PATCH] LibJS/Bytecode: Make NewArray a variable-length instruction This removes a layer of indirection in the bytecode where we had to make sure all the initializer elements were laid out in sequential registers. Array expressions no longer clobber registers permanently, and they can be reused immediately afterwards. --- .../Libraries/LibJS/Bytecode/ASTCodegen.cpp | 79 +++++++------------ .../Libraries/LibJS/Bytecode/Generator.cpp | 9 +-- Userland/Libraries/LibJS/Bytecode/Generator.h | 2 - .../Libraries/LibJS/Bytecode/Interpreter.cpp | 9 +-- Userland/Libraries/LibJS/Bytecode/Op.h | 22 ++---- 5 files changed, 40 insertions(+), 81 deletions(-) diff --git a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp index 175d7144967..ad0b37bc23d 100644 --- a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp +++ b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp @@ -350,24 +350,18 @@ static Bytecode::CodeGenerationErrorOr> arguments_to_arr auto first_spread = find_if(arguments.begin(), arguments.end(), [](auto el) { return el.is_spread; }); - Vector registers; - Bytecode::Register args_start_reg { 0 }; + Vector args; + args.ensure_capacity(first_spread.index()); for (auto it = arguments.begin(); it != first_spread; ++it) { - auto reg = generator.allocate_sequential_register(); - registers.append(reg); - if (args_start_reg.index() == 0) - args_start_reg = reg.operand().as_register(); - } - u32 i = 0; - for (auto it = arguments.begin(); it != first_spread; ++it, ++i) { - VERIFY(it->is_spread == false); - Bytecode::Register reg { args_start_reg.index() + i }; + VERIFY(!it->is_spread); + auto reg = generator.allocate_register(); auto value = TRY(it->value->generate_bytecode(generator)).value(); - generator.emit(Bytecode::Operand(reg), value); + generator.emit(reg, value); + args.append(move(reg)); } if (first_spread.index() != 0) - generator.emit_with_extra_operand_slots(2u, dst, AK::Array { Bytecode::Operand(args_start_reg), Bytecode::Operand { Bytecode::Register { args_start_reg.index() + static_cast(first_spread.index() - 1) } } }); + generator.emit_with_extra_operand_slots(args.size(), dst, args); else generator.emit(dst); @@ -1037,27 +1031,22 @@ Bytecode::CodeGenerationErrorOr> ArrayExpression::genera auto first_spread = find_if(m_elements.begin(), m_elements.end(), [](auto el) { return el && is(*el); }); - Vector registers; - Optional args_start_reg; + Vector args; + args.ensure_capacity(m_elements.size()); for (auto it = m_elements.begin(); it != first_spread; ++it) { - auto reg = generator.allocate_sequential_register(); - registers.append(reg); - if (!args_start_reg.has_value()) - args_start_reg = reg.operand().as_register(); - } - u32 i = 0; - for (auto it = m_elements.begin(); it != first_spread; ++it, ++i) { - Bytecode::Register reg { args_start_reg->index() + i }; if (*it) { + auto reg = generator.allocate_register(); auto value = TRY((*it)->generate_bytecode(generator)).value(); generator.emit(Bytecode::Operand(reg), value); + args.append(move(reg)); + } else { + args.append(generator.add_constant(Value())); } } auto dst = choose_dst(generator, preferred_dst); if (first_spread.index() != 0) { - auto reg = Bytecode::Register { args_start_reg->index() + static_cast(first_spread.index() - 1) }; - generator.emit_with_extra_operand_slots(2u, dst, AK::Array { Bytecode::Operand(*args_start_reg), Bytecode::Operand(reg) }); + generator.emit_with_extra_operand_slots(args.size(), dst, args); } else { generator.emit(dst); } @@ -1623,6 +1612,7 @@ Bytecode::CodeGenerationErrorOr> CallExpression::generat generator.emit(call_type, dst, callee, this_value, arguments, expression_string_index); } else { Vector argument_operands; + argument_operands.ensure_capacity(arguments().size()); for (auto const& argument : arguments()) { auto argument_value = TRY(argument.value->generate_bytecode(generator)).value(); auto temporary = generator.allocate_register(); @@ -1857,7 +1847,7 @@ Bytecode::CodeGenerationErrorOr> YieldExpression::genera // i. Let innerResult be ? Call(iteratorRecord.[[NextMethod]], iteratorRecord.[[Iterator]], « received.[[Value]] »). auto array = generator.allocate_register(); - generator.emit_with_extra_operand_slots(2u, array, AK::Array { received_completion_value.operand(), received_completion_value.operand() }); + generator.emit_with_extra_operand_slots(1, array, ReadonlySpan { &received_completion_value, 1 }); auto inner_result = generator.allocate_register(); generator.emit(Bytecode::Op::CallType::Call, inner_result, next_method, iterator, array); @@ -1949,7 +1939,7 @@ Bytecode::CodeGenerationErrorOr> YieldExpression::genera // 1. Let innerResult be ? Call(throw, iterator, « received.[[Value]] »). auto received_value_array = generator.allocate_register(); - generator.emit_with_extra_operand_slots(2u, received_value_array, AK::Array { received_completion_value.operand(), received_completion_value.operand() }); + generator.emit_with_extra_operand_slots(1, received_value_array, ReadonlySpan { &received_completion_value, 1 }); generator.emit(Bytecode::Op::CallType::Call, inner_result, throw_method, iterator, received_value_array); // 2. If generatorKind is async, set innerResult to ? Await(innerResult). @@ -2046,7 +2036,7 @@ Bytecode::CodeGenerationErrorOr> YieldExpression::genera // iv. Let innerReturnResult be ? Call(return, iterator, « received.[[Value]] »). auto call_array = generator.allocate_register(); - generator.emit_with_extra_operand_slots(2, call_array, AK::Array { received_completion_value.operand(), received_completion_value.operand() }); + generator.emit_with_extra_operand_slots(1, call_array, ReadonlySpan { &received_completion_value, 1 }); auto inner_return_result = generator.allocate_register(); generator.emit(Bytecode::Op::CallType::Call, inner_return_result, return_method, iterator, call_array); @@ -2306,8 +2296,6 @@ Bytecode::CodeGenerationErrorOr> TaggedTemplateLiteral:: Bytecode::Generator::SourceLocationScope scope(generator, *this); auto tag = TRY(m_tag->generate_bytecode(generator)).value(); - // FIXME: We only need to record the first and last register, - // due to packing everything in an array, same goes for argument_regs // FIXME: Follow // 13.2.8.3 GetTemplateObject ( templateLiteral ), https://tc39.es/ecma262/#sec-gettemplateobject // more closely, namely: @@ -2316,70 +2304,61 @@ Bytecode::CodeGenerationErrorOr> TaggedTemplateLiteral:: // * freeze array and raw member Vector string_regs; auto& expressions = m_template_literal->expressions(); - for (size_t i = 0; i < expressions.size(); ++i) { - if (i % 2 != 0) - continue; - string_regs.append(generator.allocate_sequential_register()); - } - size_t reg_index = 0; for (size_t i = 0; i < expressions.size(); ++i) { if (i % 2 != 0) continue; // NOTE: If the string contains invalid escapes we get a null expression here, // which we then convert to the expected `undefined` TV. See // 12.9.6.1 Static Semantics: TV, https://tc39.es/ecma262/#sec-static-semantics-tv - auto string_reg = string_regs[reg_index++]; + auto string_reg = generator.allocate_register(); if (is(expressions[i])) { generator.emit(string_reg, generator.add_constant(js_undefined())); } else { auto value = TRY(expressions[i]->generate_bytecode(generator)).value(); generator.emit(string_reg, value); } + string_regs.append(move(string_reg)); } - auto strings_array = generator.allocate_sequential_register(); + auto strings_array = generator.allocate_register(); if (string_regs.is_empty()) { generator.emit(strings_array); } else { - generator.emit_with_extra_operand_slots(2u, strings_array, AK::Array { string_regs.first().operand(), string_regs.last().operand() }); + generator.emit_with_extra_operand_slots(string_regs.size(), strings_array, string_regs); } Vector argument_regs; argument_regs.append(strings_array); - for (size_t i = 1; i < expressions.size(); i += 2) - argument_regs.append(generator.allocate_sequential_register()); for (size_t i = 1; i < expressions.size(); i += 2) { - auto string_reg = argument_regs[1 + i / 2]; + auto string_reg = generator.allocate_register(); auto string = TRY(expressions[i]->generate_bytecode(generator)).value(); generator.emit(string_reg, string); + argument_regs.append(move(string_reg)); } Vector raw_string_regs; - for ([[maybe_unused]] auto& raw_string : m_template_literal->raw_strings()) - string_regs.append(generator.allocate_sequential_register()); - - reg_index = 0; + raw_string_regs.ensure_capacity(m_template_literal->raw_strings().size()); for (auto& raw_string : m_template_literal->raw_strings()) { auto value = TRY(raw_string->generate_bytecode(generator)).value(); - auto raw_string_reg = string_regs[reg_index++]; + auto raw_string_reg = generator.allocate_register(); generator.emit(raw_string_reg, value); - raw_string_regs.append(raw_string_reg); + raw_string_regs.append(move(raw_string_reg)); } auto raw_strings_array = generator.allocate_register(); if (raw_string_regs.is_empty()) { generator.emit(raw_strings_array); } else { - generator.emit_with_extra_operand_slots(2u, raw_strings_array, AK::Array { raw_string_regs.first().operand(), raw_string_regs.last().operand() }); + generator.emit_with_extra_operand_slots(raw_string_regs.size(), raw_strings_array, raw_string_regs); } generator.emit(strings_array, generator.intern_identifier("raw"), raw_strings_array, Bytecode::Op::PropertyKind::KeyValue, generator.next_property_lookup_cache()); auto arguments = generator.allocate_register(); if (!argument_regs.is_empty()) - generator.emit_with_extra_operand_slots(2, arguments, AK::Array { argument_regs.first().operand(), argument_regs.last().operand() }); + generator.emit_with_extra_operand_slots(argument_regs.size(), arguments, argument_regs); else generator.emit(arguments); diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.cpp b/Userland/Libraries/LibJS/Bytecode/Generator.cpp index a324ec97806..696d54ba553 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Generator.cpp @@ -220,18 +220,13 @@ void Generator::grow(size_t additional_size) m_current_basic_block->grow(additional_size); } -ScopedOperand Generator::allocate_sequential_register() -{ - VERIFY(m_next_register != NumericLimits::max()); - return ScopedOperand { *this, Operand { Register { m_next_register++ } } }; -} - ScopedOperand Generator::allocate_register() { if (!m_free_registers.is_empty()) { return ScopedOperand { *this, Operand { m_free_registers.take_last() } }; } - return allocate_sequential_register(); + VERIFY(m_next_register != NumericLimits::max()); + return ScopedOperand { *this, Operand { Register { m_next_register++ } } }; } void Generator::free_register(Register reg) diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.h b/Userland/Libraries/LibJS/Bytecode/Generator.h index 7da2e117f82..5002ab3ef5f 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.h +++ b/Userland/Libraries/LibJS/Bytecode/Generator.h @@ -34,8 +34,6 @@ public: }; static CodeGenerationErrorOr> generate(VM&, ASTNode const&, ReadonlySpan parameters, FunctionKind = FunctionKind::Normal); - [[nodiscard]] ScopedOperand allocate_sequential_register(); - [[nodiscard]] ScopedOperand allocate_register(); [[nodiscard]] ScopedOperand local(u32 local_index); [[nodiscard]] ScopedOperand accumulator(); diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp index 396403da219..8046973a253 100644 --- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp @@ -86,7 +86,7 @@ static ByteString format_operand_list(StringView name, ReadonlySpan ope { StringBuilder builder; if (!name.is_empty()) - builder.appendff(", \033[32m{}\033[0m:[", name); + builder.appendff("\033[32m{}\033[0m:[", name); for (size_t i = 0; i < operands.size(); ++i) { if (i != 0) builder.append(", "sv); @@ -1095,8 +1095,7 @@ ThrowCompletionOr NewArray::execute_impl(Bytecode::Interpreter& interprete { auto array = MUST(Array::create(interpreter.realm(), 0)); for (size_t i = 0; i < m_element_count; i++) { - auto& value = interpreter.reg(Register(m_elements[0].index() + i)); - array->indexed_properties().put(i, value, default_attributes); + array->indexed_properties().put(i, interpreter.get(m_elements[i]), default_attributes); } interpreter.set(dst(), array); return {}; @@ -1883,7 +1882,7 @@ ByteString NewArray::to_byte_string_impl(Bytecode::Executable const& executable) StringBuilder builder; builder.appendff("NewArray {}", format_operand("dst"sv, dst(), executable)); if (m_element_count != 0) { - builder.appendff(", [{}-{}]", format_operand("from"sv, m_elements[0], executable), format_operand("to"sv, m_elements[1], executable)); + builder.appendff(", {}", format_operand_list("args"sv, { m_elements, m_element_count }, executable)); } return builder.to_byte_string(); } @@ -2169,7 +2168,7 @@ ByteString Call::to_byte_string_impl(Bytecode::Executable const& executable) con auto type = call_type_to_string(m_type); StringBuilder builder; - builder.appendff("Call{} {}, {}, {}"sv, + builder.appendff("Call{} {}, {}, {}, "sv, type, format_operand("dst"sv, m_dst, executable), format_operand("callee"sv, m_callee, executable), diff --git a/Userland/Libraries/LibJS/Bytecode/Op.h b/Userland/Libraries/LibJS/Bytecode/Op.h index 671983d5545..df2d7f6539d 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.h +++ b/Userland/Libraries/LibJS/Bytecode/Op.h @@ -254,13 +254,13 @@ public: { } - NewArray(Operand dst, AK::Array const& elements_range) + NewArray(Operand dst, ReadonlySpan elements) : Instruction(Type::NewArray) , m_dst(dst) - , m_element_count(elements_range[1].index() - elements_range[0].index() + 1) + , m_element_count(elements.size()) { - m_elements[0] = elements_range[0]; - m_elements[1] = elements_range[1]; + for (size_t i = 0; i < m_element_count; ++i) + m_elements[i] = elements[i]; } ThrowCompletionOr execute_impl(Bytecode::Interpreter&) const; @@ -270,19 +270,7 @@ public: size_t length_impl() const { - return round_up_to_power_of_two(alignof(void*), sizeof(*this) + sizeof(Operand) * (m_element_count == 0 ? 0 : 2)); - } - - Operand start() const - { - VERIFY(m_element_count); - return m_elements[0]; - } - - Operand end() const - { - VERIFY(m_element_count); - return m_elements[1]; + return round_up_to_power_of_two(alignof(void*), sizeof(*this) + sizeof(Operand) * m_element_count); } size_t element_count() const { return m_element_count; }