From 01e8f0889acf9fc0c84402793ecf96859c737f9a Mon Sep 17 00:00:00 2001 From: Ali Mohammad Pur Date: Wed, 9 Jun 2021 06:49:58 +0430 Subject: [PATCH] LibJS: Generate bytecode in basic blocks instead of one big block This limits the size of each block (currently set to 1K), and gets us closer to a canonical, more easily analysable bytecode format. As a result of this, "Labels" are now simply entries to basic blocks. Since there is no more 'conditional' jump (as all jumps are always taken), JumpIf{True,False} are unified to JumpConditional, and JumpIfNullish is renamed to JumpNullish. Also fixes #7914 as a result of reimplementing the loop logic. --- .../Libraries/LibJS/Bytecode/ASTCodegen.cpp | 244 +++++++++++++++--- .../Bytecode/{Block.cpp => BasicBlock.cpp} | 21 +- .../LibJS/Bytecode/{Block.h => BasicBlock.h} | 23 +- .../Libraries/LibJS/Bytecode/Generator.cpp | 26 +- Userland/Libraries/LibJS/Bytecode/Generator.h | 45 +++- .../Libraries/LibJS/Bytecode/Instruction.h | 7 +- .../Libraries/LibJS/Bytecode/Interpreter.cpp | 42 ++- .../Libraries/LibJS/Bytecode/Interpreter.h | 11 +- Userland/Libraries/LibJS/Bytecode/Label.h | 11 +- Userland/Libraries/LibJS/Bytecode/Op.cpp | 57 ++-- Userland/Libraries/LibJS/Bytecode/Op.h | 46 ++-- Userland/Libraries/LibJS/CMakeLists.txt | 2 +- Userland/Libraries/LibJS/Forward.h | 2 +- .../LibJS/Runtime/ScriptFunction.cpp | 12 +- .../Libraries/LibJS/Runtime/ScriptFunction.h | 3 +- Userland/Utilities/js.cpp | 14 +- 16 files changed, 392 insertions(+), 174 deletions(-) rename Userland/Libraries/LibJS/Bytecode/{Block.cpp => BasicBlock.cpp} (80%) rename Userland/Libraries/LibJS/Bytecode/{Block.h => BasicBlock.h} (76%) diff --git a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp index 0c593e4367..c72fd47110 100644 --- a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp +++ b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp @@ -119,23 +119,43 @@ void LogicalExpression::generate_bytecode(Bytecode::Generator& generator) const { m_lhs->generate_bytecode(generator); - Bytecode::Op::Jump* test_instr; + // lhs + // jump op (true) end (false) rhs + // rhs + // jump always (true) end + // end + + auto& rhs_block = generator.make_block(); + auto& end_block = generator.make_block(); + switch (m_op) { case LogicalOp::And: - test_instr = &generator.emit(); + generator.emit().set_targets( + Bytecode::Label { rhs_block }, + Bytecode::Label { end_block }); break; case LogicalOp::Or: - test_instr = &generator.emit(); + generator.emit().set_targets( + Bytecode::Label { end_block }, + Bytecode::Label { rhs_block }); break; case LogicalOp::NullishCoalescing: - test_instr = &generator.emit(); + generator.emit().set_targets( + Bytecode::Label { rhs_block }, + Bytecode::Label { end_block }); break; default: VERIFY_NOT_REACHED(); } + generator.switch_to_basic_block(rhs_block); m_rhs->generate_bytecode(generator); - test_instr->set_target(generator.make_label()); + + generator.emit().set_targets( + Bytecode::Label { end_block }, + {}); + + generator.switch_to_basic_block(end_block); } void UnaryExpression::generate_bytecode(Bytecode::Generator& generator) const @@ -279,58 +299,154 @@ void AssignmentExpression::generate_bytecode(Bytecode::Generator& generator) con void WhileStatement::generate_bytecode(Bytecode::Generator& generator) const { + // test + // jump if_false (true) end (false) body + // body + // jump always (true) test + // end + auto& test_block = generator.make_block(); + auto& body_block = generator.make_block(); + auto& end_block = generator.make_block(); + + // Init result register generator.emit(js_undefined()); - generator.begin_continuable_scope(); - auto test_label = generator.make_label(); auto result_reg = generator.allocate_register(); generator.emit(result_reg); + + // jump to the test block + generator.emit().set_targets( + Bytecode::Label { test_block }, + {}); + + generator.switch_to_basic_block(test_block); m_test->generate_bytecode(generator); - auto& test_jump = generator.emit(); + generator.emit().set_targets( + Bytecode::Label { body_block }, + Bytecode::Label { end_block }); + + generator.switch_to_basic_block(body_block); + generator.begin_continuable_scope(Bytecode::Label { test_block }); m_body->generate_bytecode(generator); - generator.emit(test_label); - test_jump.set_target(generator.make_label()); + generator.emit().set_targets( + Bytecode::Label { test_block }, + {}); generator.end_continuable_scope(); + + generator.switch_to_basic_block(end_block); generator.emit(result_reg); } void DoWhileStatement::generate_bytecode(Bytecode::Generator& generator) const { + // jump always (true) body + // test + // jump if_false (true) end (false) body + // body + // jump always (true) test + // end + auto& test_block = generator.make_block(); + auto& body_block = generator.make_block(); + auto& end_block = generator.make_block(); + + // Init result register generator.emit(js_undefined()); - generator.begin_continuable_scope(); - auto head_label = generator.make_label(); - m_body->generate_bytecode(generator); - generator.end_continuable_scope(); auto result_reg = generator.allocate_register(); generator.emit(result_reg); + + // jump to the body block + generator.emit().set_targets( + Bytecode::Label { body_block }, + {}); + + generator.switch_to_basic_block(test_block); m_test->generate_bytecode(generator); - generator.emit(head_label); + generator.emit().set_targets( + Bytecode::Label { body_block }, + Bytecode::Label { end_block }); + + generator.switch_to_basic_block(body_block); + generator.begin_continuable_scope(Bytecode::Label { test_block }); + m_body->generate_bytecode(generator); + generator.emit().set_targets( + Bytecode::Label { test_block }, + {}); + generator.end_continuable_scope(); + + generator.switch_to_basic_block(end_block); generator.emit(result_reg); } void ForStatement::generate_bytecode(Bytecode::Generator& generator) const { - Bytecode::Op::Jump* test_jump { nullptr }; + // init + // jump always (true) test + // test + // jump if_true (true) body (false) end + // body + // jump always (true) update + // update + // jump always (true) test + // end + + // If 'test' is missing, fuse the 'test' and 'body' basic blocks + // If 'update' is missing, fuse the 'body' and 'update' basic blocks + + Bytecode::BasicBlock* test_block_ptr { nullptr }; + Bytecode::BasicBlock* body_block_ptr { nullptr }; + Bytecode::BasicBlock* update_block_ptr { nullptr }; + + auto& end_block = generator.make_block(); if (m_init) m_init->generate_bytecode(generator); + body_block_ptr = &generator.make_block(); + + if (m_test) + test_block_ptr = &generator.make_block(); + else + test_block_ptr = body_block_ptr; + + if (m_update) + update_block_ptr = &generator.make_block(); + else + update_block_ptr = body_block_ptr; + generator.emit(js_undefined()); - generator.begin_continuable_scope(); - auto jump_label = generator.make_label(); auto result_reg = generator.allocate_register(); generator.emit(result_reg); + + generator.emit().set_targets( + Bytecode::Label { *test_block_ptr }, + {}); + if (m_test) { + generator.switch_to_basic_block(*test_block_ptr); m_test->generate_bytecode(generator); - test_jump = &generator.emit(); + generator.emit().set_targets( + Bytecode::Label { *body_block_ptr }, + Bytecode::Label { end_block }); } + generator.switch_to_basic_block(*body_block_ptr); + generator.begin_continuable_scope(Bytecode::Label { *update_block_ptr }); m_body->generate_bytecode(generator); - if (m_update) - m_update->generate_bytecode(generator); - generator.emit(jump_label); - if (m_test) - test_jump->set_target(generator.make_label()); generator.end_continuable_scope(); + + if (m_update) { + generator.emit().set_targets( + Bytecode::Label { *update_block_ptr }, + {}); + + generator.switch_to_basic_block(*update_block_ptr); + m_update->generate_bytecode(generator); + } + + generator.emit().set_targets( + Bytecode::Label { *test_block_ptr }, + {}); + + generator.switch_to_basic_block(end_block); generator.emit(result_reg); } @@ -408,23 +524,60 @@ void ReturnStatement::generate_bytecode(Bytecode::Generator& generator) const void IfStatement::generate_bytecode(Bytecode::Generator& generator) const { - m_predicate->generate_bytecode(generator); - auto& else_jump = generator.emit(); + // test + // jump if_true (true) true (false) false + // true + // jump always (true) end + // false + // jump always (true) end + // end + // If the 'false' branch doesn't exist, we're just gonna substitute it for 'end' and elide the last two entries above. + + auto& true_block = generator.make_block(); + auto& false_block = generator.make_block(); + + m_predicate->generate_bytecode(generator); + generator.emit().set_targets( + Bytecode::Label { true_block }, + Bytecode::Label { false_block }); + + Bytecode::Op::Jump* true_block_jump { nullptr }; + + generator.switch_to_basic_block(true_block); m_consequent->generate_bytecode(generator); + if (!generator.is_current_block_terminated()) + true_block_jump = &generator.emit(); + + generator.switch_to_basic_block(false_block); if (m_alternate) { - auto& if_jump = generator.emit(); - else_jump.set_target(generator.make_label()); + auto& end_block = generator.make_block(); + m_alternate->generate_bytecode(generator); - if_jump.set_target(generator.make_label()); + if (!generator.is_current_block_terminated()) + generator.emit().set_targets( + Bytecode::Label { end_block }, + {}); + + if (true_block_jump) + true_block_jump->set_targets( + Bytecode::Label { end_block }, + {}); + + generator.switch_to_basic_block(end_block); } else { - else_jump.set_target(generator.make_label()); + if (true_block_jump) + true_block_jump->set_targets( + Bytecode::Label { false_block }, + {}); } } void ContinueStatement::generate_bytecode(Bytecode::Generator& generator) const { - generator.emit(generator.nearest_continuable_scope()); + generator.emit().set_targets( + generator.nearest_continuable_scope(), + {}); } void DebuggerStatement::generate_bytecode(Bytecode::Generator&) const @@ -433,16 +586,36 @@ void DebuggerStatement::generate_bytecode(Bytecode::Generator&) const void ConditionalExpression::generate_bytecode(Bytecode::Generator& generator) const { + // test + // jump if_true (true) true (false) false + // true + // jump always (true) end + // false + // jump always (true) end + // end + + auto& true_block = generator.make_block(); + auto& false_block = generator.make_block(); + auto& end_block = generator.make_block(); + m_test->generate_bytecode(generator); - auto& alternate_jump = generator.emit(); + generator.emit().set_targets( + Bytecode::Label { true_block }, + Bytecode::Label { false_block }); + generator.switch_to_basic_block(true_block); m_consequent->generate_bytecode(generator); - auto& end_jump = generator.emit(); + generator.emit().set_targets( + Bytecode::Label { end_block }, + {}); - alternate_jump.set_target(generator.make_label()); + generator.switch_to_basic_block(false_block); m_alternate->generate_bytecode(generator); + generator.emit().set_targets( + Bytecode::Label { end_block }, + {}); - end_jump.set_target(generator.make_label()); + generator.switch_to_basic_block(end_block); } void SequenceExpression::generate_bytecode(Bytecode::Generator& generator) const @@ -464,5 +637,4 @@ void TemplateLiteral::generate_bytecode(Bytecode::Generator& generator) const } } } - } diff --git a/Userland/Libraries/LibJS/Bytecode/Block.cpp b/Userland/Libraries/LibJS/Bytecode/BasicBlock.cpp similarity index 80% rename from Userland/Libraries/LibJS/Bytecode/Block.cpp rename to Userland/Libraries/LibJS/Bytecode/BasicBlock.cpp index ef0add31a1..072f6aa78e 100644 --- a/Userland/Libraries/LibJS/Bytecode/Block.cpp +++ b/Userland/Libraries/LibJS/Bytecode/BasicBlock.cpp @@ -5,28 +5,29 @@ */ #include -#include +#include #include #include namespace JS::Bytecode { -NonnullOwnPtr Block::create() +NonnullOwnPtr BasicBlock::create(String name) { - return adopt_own(*new Block); + return adopt_own(*new BasicBlock(move(name))); } -Block::Block() +BasicBlock::BasicBlock(String name) + : m_name(move(name)) { // FIXME: This is not the smartest solution ever. Find something cleverer! // The main issue we're working around here is that we don't want pointers into the bytecode stream to become invalidated // during code generation due to dynamic buffer resizing. Otherwise we could just use a Vector. - m_buffer_capacity = 64 * KiB; + m_buffer_capacity = 1 * KiB; m_buffer = (u8*)mmap(nullptr, m_buffer_capacity, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, 0, 0); VERIFY(m_buffer != MAP_FAILED); } -Block::~Block() +BasicBlock::~BasicBlock() { Bytecode::InstructionStreamIterator it(instruction_stream()); while (!it.at_end()) { @@ -38,7 +39,7 @@ Block::~Block() munmap(m_buffer, m_buffer_capacity); } -void Block::seal() +void BasicBlock::seal() { // FIXME: mprotect the instruction stream as PROT_READ // This is currently not possible because instructions can have destructors (that clean up strings) @@ -46,16 +47,18 @@ void Block::seal() // It also doesn't work because instructions that have String members use RefPtr internally which must be in writable memory. } -void Block::dump() const +void BasicBlock::dump() const { Bytecode::InstructionStreamIterator it(instruction_stream()); + if (!m_name.is_empty()) + warnln("{}:", m_name); while (!it.at_end()) { warnln("[{:4x}] {}", it.offset(), (*it).to_string()); ++it; } } -void Block::grow(size_t additional_size) +void BasicBlock::grow(size_t additional_size) { m_buffer_size += additional_size; VERIFY(m_buffer_size <= m_buffer_capacity); diff --git a/Userland/Libraries/LibJS/Bytecode/Block.h b/Userland/Libraries/LibJS/Bytecode/BasicBlock.h similarity index 76% rename from Userland/Libraries/LibJS/Bytecode/Block.h rename to Userland/Libraries/LibJS/Bytecode/BasicBlock.h index 4e81d2b1d3..0601b89a93 100644 --- a/Userland/Libraries/LibJS/Bytecode/Block.h +++ b/Userland/Libraries/LibJS/Bytecode/BasicBlock.h @@ -8,6 +8,7 @@ #include #include +#include #include namespace JS::Bytecode { @@ -37,30 +38,32 @@ private: size_t m_offset { 0 }; }; -class Block { +class BasicBlock { public: - static NonnullOwnPtr create(); - ~Block(); + static NonnullOwnPtr create(String name); + ~BasicBlock(); void seal(); void dump() const; ReadonlyBytes instruction_stream() const { return ReadonlyBytes { m_buffer, m_buffer_size }; } - size_t register_count() const { return m_register_count; } - - void set_register_count(Badge, size_t count) { m_register_count = count; } - void* next_slot() { return m_buffer + m_buffer_size; } void grow(size_t additional_size); -private: - Block(); + void terminate(Badge) { m_is_terminated = true; } + bool is_terminated() const { return m_is_terminated; } + + String const& name() const { return m_name; } + +private: + BasicBlock(String name); - size_t m_register_count { 0 }; u8* m_buffer { nullptr }; size_t m_buffer_capacity { 0 }; size_t m_buffer_size { 0 }; + bool m_is_terminated { false }; + String m_name; }; } diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.cpp b/Userland/Libraries/LibJS/Bytecode/Generator.cpp index 2d2ad73520..1bdd097312 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Generator.cpp @@ -4,9 +4,8 @@ * SPDX-License-Identifier: BSD-2-Clause */ -#include #include -#include +#include #include #include #include @@ -16,30 +15,30 @@ namespace JS::Bytecode { Generator::Generator() { - m_block = Block::create(); } Generator::~Generator() { } -OwnPtr Generator::generate(ASTNode const& node) +ExecutionUnit Generator::generate(ASTNode const& node) { Generator generator; + generator.switch_to_basic_block(generator.make_block()); node.generate_bytecode(generator); - generator.m_block->set_register_count({}, generator.m_next_register); - generator.m_block->seal(); - return move(generator.m_block); + return { move(generator.m_root_basic_blocks), generator.m_next_register }; } void Generator::grow(size_t additional_size) { - m_block->grow(additional_size); + VERIFY(m_current_basic_block); + m_current_basic_block->grow(additional_size); } void* Generator::next_slot() { - return m_block->next_slot(); + VERIFY(m_current_basic_block); + return m_current_basic_block->next_slot(); } Register Generator::allocate_register() @@ -48,19 +47,14 @@ Register Generator::allocate_register() return Register { m_next_register++ }; } -Label Generator::make_label() const -{ - return Label { m_block->instruction_stream().size() }; -} - Label Generator::nearest_continuable_scope() const { return m_continuable_scopes.last(); } -void Generator::begin_continuable_scope() +void Generator::begin_continuable_scope(Label continue_target) { - m_continuable_scopes.append(make_label()); + m_continuable_scopes.append(continue_target); } void Generator::end_continuable_scope() diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.h b/Userland/Libraries/LibJS/Bytecode/Generator.h index 88bac76ebe..bf59eec66b 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.h +++ b/Userland/Libraries/LibJS/Bytecode/Generator.h @@ -6,43 +6,73 @@ #pragma once +#include #include +#include +#include #include #include #include namespace JS::Bytecode { +struct ExecutionUnit { + NonnullOwnPtrVector basic_blocks; + size_t number_of_registers { 0 }; +}; + class Generator { public: - static OwnPtr generate(ASTNode const&); + static ExecutionUnit generate(ASTNode const&); Register allocate_register(); template OpType& emit(Args&&... args) { + VERIFY(!is_current_block_terminated()); void* slot = next_slot(); grow(sizeof(OpType)); new (slot) OpType(forward(args)...); + if constexpr (OpType::IsTerminator) + m_current_basic_block->terminate({}); return *static_cast(slot); } template OpType& emit_with_extra_register_slots(size_t extra_register_slots, Args&&... args) { + VERIFY(!is_current_block_terminated()); void* slot = next_slot(); grow(sizeof(OpType) + extra_register_slots * sizeof(Register)); new (slot) OpType(forward(args)...); + if constexpr (OpType::IsTerminator) + m_current_basic_block->terminate({}); return *static_cast(slot); } - Label make_label() const; - - void begin_continuable_scope(); + void begin_continuable_scope(Label continue_target); void end_continuable_scope(); - Label nearest_continuable_scope() const; + [[nodiscard]] Label nearest_continuable_scope() const; + + void switch_to_basic_block(BasicBlock& block) + { + m_current_basic_block = █ + } + + BasicBlock& make_block(String name = {}) + { + if (name.is_empty()) + name = String::number(m_next_block++); + m_root_basic_blocks.append(BasicBlock::create(name)); + return m_root_basic_blocks.last(); + } + + bool is_current_block_terminated() const + { + return m_current_basic_block->is_terminated(); + } private: Generator(); @@ -51,8 +81,11 @@ private: void grow(size_t); void* next_slot(); - OwnPtr m_block; + BasicBlock* m_current_basic_block { nullptr }; + NonnullOwnPtrVector m_root_basic_blocks; + u32 m_next_register { 1 }; + u32 m_next_block { 1 }; Vector