Просмотр исходного кода

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.
Ali Mohammad Pur 4 лет назад
Родитель
Сommit
01e8f0889a

+ 207 - 35
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<Bytecode::Op::JumpIfFalse>();
+        generator.emit<Bytecode::Op::JumpConditional>().set_targets(
+            Bytecode::Label { rhs_block },
+            Bytecode::Label { end_block });
         break;
     case LogicalOp::Or:
-        test_instr = &generator.emit<Bytecode::Op::JumpIfTrue>();
+        generator.emit<Bytecode::Op::JumpConditional>().set_targets(
+            Bytecode::Label { end_block },
+            Bytecode::Label { rhs_block });
         break;
     case LogicalOp::NullishCoalescing:
-        test_instr = &generator.emit<Bytecode::Op::JumpIfNotNullish>();
+        generator.emit<Bytecode::Op::JumpNullish>().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<Bytecode::Op::Jump>().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<Bytecode::Op::LoadImmediate>(js_undefined());
-    generator.begin_continuable_scope();
-    auto test_label = generator.make_label();
     auto result_reg = generator.allocate_register();
     generator.emit<Bytecode::Op::Store>(result_reg);
+
+    // jump to the test block
+    generator.emit<Bytecode::Op::Jump>().set_targets(
+        Bytecode::Label { test_block },
+        {});
+
+    generator.switch_to_basic_block(test_block);
     m_test->generate_bytecode(generator);
-    auto& test_jump = generator.emit<Bytecode::Op::JumpIfFalse>();
+    generator.emit<Bytecode::Op::JumpConditional>().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<Bytecode::Op::Jump>(test_label);
-    test_jump.set_target(generator.make_label());
+    generator.emit<Bytecode::Op::Jump>().set_targets(
+        Bytecode::Label { test_block },
+        {});
     generator.end_continuable_scope();
+
+    generator.switch_to_basic_block(end_block);
     generator.emit<Bytecode::Op::Load>(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<Bytecode::Op::LoadImmediate>(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<Bytecode::Op::Store>(result_reg);
+
+    // jump to the body block
+    generator.emit<Bytecode::Op::Jump>().set_targets(
+        Bytecode::Label { body_block },
+        {});
+
+    generator.switch_to_basic_block(test_block);
     m_test->generate_bytecode(generator);
-    generator.emit<Bytecode::Op::JumpIfTrue>(head_label);
+    generator.emit<Bytecode::Op::JumpConditional>().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<Bytecode::Op::Jump>().set_targets(
+        Bytecode::Label { test_block },
+        {});
+    generator.end_continuable_scope();
+
+    generator.switch_to_basic_block(end_block);
     generator.emit<Bytecode::Op::Load>(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<Bytecode::Op::LoadImmediate>(js_undefined());
-    generator.begin_continuable_scope();
-    auto jump_label = generator.make_label();
     auto result_reg = generator.allocate_register();
     generator.emit<Bytecode::Op::Store>(result_reg);
+
+    generator.emit<Bytecode::Op::Jump>().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<Bytecode::Op::JumpIfFalse>();
+        generator.emit<Bytecode::Op::JumpConditional>().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<Bytecode::Op::Jump>(jump_label);
-    if (m_test)
-        test_jump->set_target(generator.make_label());
     generator.end_continuable_scope();
+
+    if (m_update) {
+        generator.emit<Bytecode::Op::Jump>().set_targets(
+            Bytecode::Label { *update_block_ptr },
+            {});
+
+        generator.switch_to_basic_block(*update_block_ptr);
+        m_update->generate_bytecode(generator);
+    }
+
+    generator.emit<Bytecode::Op::Jump>().set_targets(
+        Bytecode::Label { *test_block_ptr },
+        {});
+
+    generator.switch_to_basic_block(end_block);
     generator.emit<Bytecode::Op::Load>(result_reg);
 }
 
@@ -408,23 +524,60 @@ void ReturnStatement::generate_bytecode(Bytecode::Generator& generator) const
 
 void IfStatement::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
+
+    // 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);
-    auto& else_jump = generator.emit<Bytecode::Op::JumpIfFalse>();
+    generator.emit<Bytecode::Op::JumpConditional>().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<Bytecode::Op::Jump>();
+
+    generator.switch_to_basic_block(false_block);
     if (m_alternate) {
-        auto& if_jump = generator.emit<Bytecode::Op::Jump>();
-        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<Bytecode::Op::Jump>().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<Bytecode::Op::Jump>(generator.nearest_continuable_scope());
+    generator.emit<Bytecode::Op::Jump>().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<Bytecode::Op::JumpIfFalse>();
+    generator.emit<Bytecode::Op::JumpConditional>().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<Bytecode::Op::Jump>();
+    generator.emit<Bytecode::Op::Jump>().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<Bytecode::Op::Jump>().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
         }
     }
 }
-
 }

+ 12 - 9
Userland/Libraries/LibJS/Bytecode/Block.cpp → Userland/Libraries/LibJS/Bytecode/BasicBlock.cpp

@@ -5,28 +5,29 @@
  */
 
 #include <AK/String.h>
-#include <LibJS/Bytecode/Block.h>
+#include <LibJS/Bytecode/BasicBlock.h>
 #include <LibJS/Bytecode/Op.h>
 #include <sys/mman.h>
 
 namespace JS::Bytecode {
 
-NonnullOwnPtr<Block> Block::create()
+NonnullOwnPtr<BasicBlock> 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);

+ 12 - 9
Userland/Libraries/LibJS/Bytecode/Block.h → Userland/Libraries/LibJS/Bytecode/BasicBlock.h

@@ -8,6 +8,7 @@
 
 #include <AK/Badge.h>
 #include <AK/NonnullOwnPtrVector.h>
+#include <AK/String.h>
 #include <LibJS/Forward.h>
 
 namespace JS::Bytecode {
@@ -37,30 +38,32 @@ private:
     size_t m_offset { 0 };
 };
 
-class Block {
+class BasicBlock {
 public:
-    static NonnullOwnPtr<Block> create();
-    ~Block();
+    static NonnullOwnPtr<BasicBlock> 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<Bytecode::Generator>, size_t count) { m_register_count = count; }
-
     void* next_slot() { return m_buffer + m_buffer_size; }
     void grow(size_t additional_size);
 
+    void terminate(Badge<Generator>) { m_is_terminated = true; }
+    bool is_terminated() const { return m_is_terminated; }
+
+    String const& name() const { return m_name; }
+
 private:
-    Block();
+    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;
 };
 
 }

+ 10 - 16
Userland/Libraries/LibJS/Bytecode/Generator.cpp

@@ -4,9 +4,8 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include <AK/OwnPtr.h>
 #include <LibJS/AST.h>
-#include <LibJS/Bytecode/Block.h>
+#include <LibJS/Bytecode/BasicBlock.h>
 #include <LibJS/Bytecode/Generator.h>
 #include <LibJS/Bytecode/Instruction.h>
 #include <LibJS/Bytecode/Register.h>
@@ -16,30 +15,30 @@ namespace JS::Bytecode {
 
 Generator::Generator()
 {
-    m_block = Block::create();
 }
 
 Generator::~Generator()
 {
 }
 
-OwnPtr<Block> 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()

+ 39 - 6
Userland/Libraries/LibJS/Bytecode/Generator.h

@@ -6,43 +6,73 @@
 
 #pragma once
 
+#include <AK/NonnullOwnPtrVector.h>
 #include <AK/OwnPtr.h>
+#include <AK/SinglyLinkedList.h>
+#include <LibJS/Bytecode/BasicBlock.h>
 #include <LibJS/Bytecode/Label.h>
 #include <LibJS/Bytecode/Register.h>
 #include <LibJS/Forward.h>
 
 namespace JS::Bytecode {
 
+struct ExecutionUnit {
+    NonnullOwnPtrVector<BasicBlock> basic_blocks;
+    size_t number_of_registers { 0 };
+};
+
 class Generator {
 public:
-    static OwnPtr<Block> generate(ASTNode const&);
+    static ExecutionUnit generate(ASTNode const&);
 
     Register allocate_register();
 
     template<typename OpType, typename... Args>
     OpType& emit(Args&&... args)
     {
+        VERIFY(!is_current_block_terminated());
         void* slot = next_slot();
         grow(sizeof(OpType));
         new (slot) OpType(forward<Args>(args)...);
+        if constexpr (OpType::IsTerminator)
+            m_current_basic_block->terminate({});
         return *static_cast<OpType*>(slot);
     }
 
     template<typename OpType, typename... Args>
     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>(args)...);
+        if constexpr (OpType::IsTerminator)
+            m_current_basic_block->terminate({});
         return *static_cast<OpType*>(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 = &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<Block> m_block;
+    BasicBlock* m_current_basic_block { nullptr };
+    NonnullOwnPtrVector<BasicBlock> m_root_basic_blocks;
+
     u32 m_next_register { 1 };
+    u32 m_next_block { 1 };
     Vector<Label> m_continuable_scopes;
 };
 

+ 4 - 3
Userland/Libraries/LibJS/Bytecode/Instruction.h

@@ -36,9 +36,8 @@
     O(PutById)                    \
     O(GetById)                    \
     O(Jump)                       \
-    O(JumpIfFalse)                \
-    O(JumpIfTrue)                 \
-    O(JumpIfNotNullish)           \
+    O(JumpConditional)            \
+    O(JumpNullish)                \
     O(Call)                       \
     O(EnterScope)                 \
     O(Return)                     \
@@ -61,6 +60,8 @@ namespace JS::Bytecode {
 
 class Instruction {
 public:
+    constexpr static bool IsTerminator = false;
+
     enum class Type {
 #define __BYTECODE_OP(op) \
     op,

+ 29 - 15
Userland/Libraries/LibJS/Bytecode/Interpreter.cpp

@@ -5,7 +5,7 @@
  */
 
 #include <AK/Debug.h>
-#include <LibJS/Bytecode/Block.h>
+#include <LibJS/Bytecode/BasicBlock.h>
 #include <LibJS/Bytecode/Instruction.h>
 #include <LibJS/Bytecode/Interpreter.h>
 #include <LibJS/Runtime/GlobalObject.h>
@@ -33,9 +33,9 @@ Interpreter::~Interpreter()
     s_current = nullptr;
 }
 
-Value Interpreter::run(Bytecode::Block const& block)
+Value Interpreter::run(ExecutionUnit const& execution_unit)
 {
-    dbgln_if(JS_BYTECODE_DEBUG, "Bytecode::Interpreter will run block {:p}", &block);
+    dbgln_if(JS_BYTECODE_DEBUG, "Bytecode::Interpreter will run unit {:p}", &execution_unit);
 
     CallFrame global_call_frame;
     if (vm().call_stack().is_empty()) {
@@ -50,23 +50,37 @@ Value Interpreter::run(Bytecode::Block const& block)
         VERIFY(!vm().exception());
     }
 
+    auto block = &execution_unit.basic_blocks.first();
     m_register_windows.append(make<RegisterWindow>());
-    registers().resize(block.register_count());
-
-    Bytecode::InstructionStreamIterator pc(block.instruction_stream());
-    while (!pc.at_end()) {
-        auto& instruction = *pc;
-        instruction.execute(*this);
-        if (m_pending_jump.has_value()) {
-            pc.jump(m_pending_jump.release_value());
-            continue;
+    registers().resize(execution_unit.number_of_registers);
+
+    for (;;) {
+        Bytecode::InstructionStreamIterator pc(block->instruction_stream());
+        bool will_jump = false;
+        bool will_return = false;
+        while (!pc.at_end()) {
+            auto& instruction = *pc;
+            instruction.execute(*this);
+            if (m_pending_jump.has_value()) {
+                block = m_pending_jump.release_value();
+                will_jump = true;
+                break;
+            }
+            if (!m_return_value.is_empty()) {
+                will_return = true;
+                break;
+            }
+            ++pc;
         }
-        if (!m_return_value.is_empty())
+
+        if (will_return)
+            break;
+
+        if (pc.at_end() && !will_jump)
             break;
-        ++pc;
     }
 
-    dbgln_if(JS_BYTECODE_DEBUG, "Bytecode::Interpreter did run block {:p}", &block);
+    dbgln_if(JS_BYTECODE_DEBUG, "Bytecode::Interpreter did run unit {:p}", &execution_unit);
 
     if constexpr (JS_BYTECODE_DEBUG) {
         for (size_t i = 0; i < registers().size(); ++i) {

+ 7 - 4
Userland/Libraries/LibJS/Bytecode/Interpreter.h

@@ -6,7 +6,7 @@
 
 #pragma once
 
-#include <AK/NonnullOwnPtrVector.h>
+#include "Generator.h"
 #include <LibJS/Bytecode/Label.h>
 #include <LibJS/Bytecode/Register.h>
 #include <LibJS/Forward.h>
@@ -28,12 +28,15 @@ public:
     GlobalObject& global_object() { return m_global_object; }
     VM& vm() { return m_vm; }
 
-    Value run(Bytecode::Block const&);
+    Value run(Bytecode::ExecutionUnit const&);
 
     ALWAYS_INLINE Value& accumulator() { return reg(Register::accumulator()); }
     Value& reg(Register const& r) { return registers()[r.index()]; }
 
-    void jump(Label const& label) { m_pending_jump = label.address(); }
+    void jump(Label const& label)
+    {
+        m_pending_jump = &label.block();
+    }
     void do_return(Value return_value) { m_return_value = return_value; }
 
 private:
@@ -42,7 +45,7 @@ private:
     VM& m_vm;
     GlobalObject& m_global_object;
     NonnullOwnPtrVector<RegisterWindow> m_register_windows;
-    Optional<size_t> m_pending_jump;
+    Optional<BasicBlock const*> m_pending_jump;
     Value m_return_value;
 };
 

+ 6 - 5
Userland/Libraries/LibJS/Bytecode/Label.h

@@ -7,20 +7,21 @@
 #pragma once
 
 #include <AK/Format.h>
+#include <LibJS/Bytecode/BasicBlock.h>
 
 namespace JS::Bytecode {
 
 class Label {
 public:
-    explicit Label(size_t address)
-        : m_address(address)
+    explicit Label(BasicBlock const& block)
+        : m_block(block)
     {
     }
 
-    size_t address() const { return m_address; }
+    auto& block() const { return m_block; }
 
 private:
-    size_t m_address { 0 };
+    BasicBlock const& m_block;
 };
 
 }
@@ -29,6 +30,6 @@ template<>
 struct AK::Formatter<JS::Bytecode::Label> : AK::Formatter<FormatString> {
     void format(FormatBuilder& builder, JS::Bytecode::Label const& value)
     {
-        return AK::Formatter<FormatString>::format(builder, "@{:x}", value.address());
+        return AK::Formatter<FormatString>::format(builder, "@{}", value.block().name());
     }
 };

+ 25 - 32
Userland/Libraries/LibJS/Bytecode/Op.cpp

@@ -174,31 +174,29 @@ void PutById::execute(Bytecode::Interpreter& interpreter) const
 
 void Jump::execute(Bytecode::Interpreter& interpreter) const
 {
-    interpreter.jump(*m_target);
+    interpreter.jump(*m_true_target);
 }
 
-void JumpIfFalse::execute(Bytecode::Interpreter& interpreter) const
+void JumpConditional::execute(Bytecode::Interpreter& interpreter) const
 {
-    VERIFY(m_target.has_value());
-    auto result = interpreter.accumulator();
-    if (!result.to_boolean())
-        interpreter.jump(m_target.value());
-}
-
-void JumpIfTrue::execute(Bytecode::Interpreter& interpreter) const
-{
-    VERIFY(m_target.has_value());
+    VERIFY(m_true_target.has_value());
+    VERIFY(m_false_target.has_value());
     auto result = interpreter.accumulator();
     if (result.to_boolean())
-        interpreter.jump(m_target.value());
+        interpreter.jump(m_true_target.value());
+    else
+        interpreter.jump(m_false_target.value());
 }
 
-void JumpIfNotNullish::execute(Bytecode::Interpreter& interpreter) const
+void JumpNullish::execute(Bytecode::Interpreter& interpreter) const
 {
-    VERIFY(m_target.has_value());
+    VERIFY(m_true_target.has_value());
+    VERIFY(m_false_target.has_value());
     auto result = interpreter.accumulator();
-    if (!result.is_nullish())
-        interpreter.jump(m_target.value());
+    if (result.is_nullish())
+        interpreter.jump(m_true_target.value());
+    else
+        interpreter.jump(m_false_target.value());
 }
 
 void Call::execute(Bytecode::Interpreter& interpreter) const
@@ -321,28 +319,23 @@ String GetById::to_string() const
 
 String Jump::to_string() const
 {
-    return String::formatted("Jump {}", *m_target);
-}
-
-String JumpIfFalse::to_string() const
-{
-    if (m_target.has_value())
-        return String::formatted("JumpIfFalse target:{}", m_target.value());
-    return "JumpIfFalse target:<empty>";
+    if (m_true_target.has_value())
+        return String::formatted("Jump {}", *m_true_target);
+    return String::formatted("Jump <empty>");
 }
 
-String JumpIfTrue::to_string() const
+String JumpConditional::to_string() const
 {
-    if (m_target.has_value())
-        return String::formatted("JumpIfTrue target:{}", m_target.value());
-    return "JumpIfTrue result:{}, target:<empty>";
+    auto true_string = m_true_target.has_value() ? String::formatted("{}", *m_true_target) : "<empty>";
+    auto false_string = m_false_target.has_value() ? String::formatted("{}", *m_false_target) : "<empty>";
+    return String::formatted("JumpConditional true:{} false:{}", true_string, false_string);
 }
 
-String JumpIfNotNullish::to_string() const
+String JumpNullish::to_string() const
 {
-    if (m_target.has_value())
-        return String::formatted("JumpIfNotNullish target:{}", m_target.value());
-    return "JumpIfNotNullish target:<empty>";
+    auto true_string = m_true_target.has_value() ? String::formatted("{}", *m_true_target) : "<empty>";
+    auto false_string = m_false_target.has_value() ? String::formatted("{}", *m_false_target) : "<empty>";
+    return String::formatted("JumpNullish null:{} nonnull:{}", true_string, false_string);
 }
 
 String Call::to_string() const

+ 23 - 23
Userland/Libraries/LibJS/Bytecode/Op.h

@@ -268,42 +268,40 @@ private:
 
 class Jump : public Instruction {
 public:
-    explicit Jump(Type type, Optional<Label> target = {})
+    constexpr static bool IsTerminator = true;
+
+    explicit Jump(Type type, Optional<Label> taken_target = {}, Optional<Label> nontaken_target = {})
         : Instruction(type)
-        , m_target(move(target))
+        , m_true_target(move(taken_target))
+        , m_false_target(move(nontaken_target))
     {
     }
 
-    explicit Jump(Optional<Label> target = {})
+    explicit Jump(Optional<Label> taken_target = {}, Optional<Label> nontaken_target = {})
         : Instruction(Type::Jump)
-        , m_target(move(target))
+        , m_true_target(move(taken_target))
+        , m_false_target(move(nontaken_target))
     {
     }
 
-    void set_target(Optional<Label> target) { m_target = move(target); }
-
-    void execute(Bytecode::Interpreter&) const;
-    String to_string() const;
-
-protected:
-    Optional<Label> m_target;
-};
-
-class JumpIfFalse final : public Jump {
-public:
-    explicit JumpIfFalse(Optional<Label> target = {})
-        : Jump(Type::JumpIfFalse, move(target))
+    void set_targets(Optional<Label> true_target, Optional<Label> false_target)
     {
+        m_true_target = move(true_target);
+        m_false_target = move(false_target);
     }
 
     void execute(Bytecode::Interpreter&) const;
     String to_string() const;
+
+protected:
+    Optional<Label> m_true_target;
+    Optional<Label> m_false_target;
 };
 
-class JumpIfTrue : public Jump {
+class JumpConditional final : public Jump {
 public:
-    explicit JumpIfTrue(Optional<Label> target = {})
-        : Jump(Type::JumpIfTrue, move(target))
+    explicit JumpConditional(Optional<Label> true_target = {}, Optional<Label> false_target = {})
+        : Jump(Type::JumpConditional, move(true_target), move(false_target))
     {
     }
 
@@ -311,10 +309,10 @@ public:
     String to_string() const;
 };
 
-class JumpIfNotNullish final : public Jump {
+class JumpNullish final : public Jump {
 public:
-    explicit JumpIfNotNullish(Optional<Label> target = {})
-        : Jump(Type::JumpIfNotNullish, move(target))
+    explicit JumpNullish(Optional<Label> true_target = {}, Optional<Label> false_target = {})
+        : Jump(Type::JumpNullish, move(true_target), move(false_target))
     {
     }
 
@@ -364,6 +362,8 @@ private:
 
 class Return final : public Instruction {
 public:
+    constexpr static bool IsTerminator = true;
+
     Return()
         : Instruction(Type::Return)
     {

+ 1 - 1
Userland/Libraries/LibJS/CMakeLists.txt

@@ -1,7 +1,7 @@
 set(SOURCES
         AST.cpp
         Bytecode/ASTCodegen.cpp
-        Bytecode/Block.cpp
+        Bytecode/BasicBlock.cpp
         Bytecode/Generator.cpp
         Bytecode/Instruction.cpp
         Bytecode/Interpreter.cpp

+ 1 - 1
Userland/Libraries/LibJS/Forward.h

@@ -163,7 +163,7 @@ template<class T>
 class Handle;
 
 namespace Bytecode {
-class Block;
+class BasicBlock;
 class Generator;
 class Instruction;
 class Interpreter;

+ 6 - 6
Userland/Libraries/LibJS/Runtime/ScriptFunction.cpp

@@ -7,7 +7,7 @@
 #include <AK/Debug.h>
 #include <AK/Function.h>
 #include <LibJS/AST.h>
-#include <LibJS/Bytecode/Block.h>
+#include <LibJS/Bytecode/BasicBlock.h>
 #include <LibJS/Bytecode/Generator.h>
 #include <LibJS/Bytecode/Interpreter.h>
 #include <LibJS/Interpreter.h>
@@ -151,15 +151,15 @@ Value ScriptFunction::execute_function_body()
 
     if (bytecode_interpreter) {
         prepare_arguments();
-        if (!m_bytecode_block) {
-            m_bytecode_block = Bytecode::Generator::generate(m_body);
-            VERIFY(m_bytecode_block);
+        if (!m_bytecode_execution_unit.has_value()) {
+            m_bytecode_execution_unit = Bytecode::Generator::generate(m_body);
             if constexpr (JS_BYTECODE_DEBUG) {
                 dbgln("Compiled Bytecode::Block for function '{}':", m_name);
-                m_bytecode_block->dump();
+                for (auto& block : m_bytecode_execution_unit->basic_blocks)
+                    block.dump();
             }
         }
-        return bytecode_interpreter->run(*m_bytecode_block);
+        return bytecode_interpreter->run(*m_bytecode_execution_unit);
     } else {
         OwnPtr<Interpreter> local_interpreter;
         ast_interpreter = vm.interpreter_if_exists();

+ 2 - 1
Userland/Libraries/LibJS/Runtime/ScriptFunction.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include <LibJS/AST.h>
+#include <LibJS/Bytecode/Generator.h>
 #include <LibJS/Runtime/Function.h>
 
 namespace JS {
@@ -47,7 +48,7 @@ private:
     FlyString m_name;
     NonnullRefPtr<Statement> m_body;
     const Vector<FunctionNode::Parameter> m_parameters;
-    OwnPtr<Bytecode::Block> m_bytecode_block;
+    Optional<Bytecode::ExecutionUnit> m_bytecode_execution_unit;
     ScopeObject* m_parent_scope { nullptr };
     i32 m_function_length { 0 };
     bool m_is_strict { false };

+ 7 - 7
Userland/Utilities/js.cpp

@@ -14,7 +14,7 @@
 #include <LibCore/File.h>
 #include <LibCore/StandardPaths.h>
 #include <LibJS/AST.h>
-#include <LibJS/Bytecode/Block.h>
+#include <LibJS/Bytecode/BasicBlock.h>
 #include <LibJS/Bytecode/Generator.h>
 #include <LibJS/Bytecode/Interpreter.h>
 #include <LibJS/Console.h>
@@ -494,15 +494,15 @@ static bool parse_and_run(JS::Interpreter& interpreter, const StringView& source
         program->dump(0);
 
     if (s_dump_bytecode || s_run_bytecode) {
-        auto block = JS::Bytecode::Generator::generate(*program);
-        VERIFY(block);
-
-        if (s_dump_bytecode)
-            block->dump();
+        auto unit = JS::Bytecode::Generator::generate(*program);
+        if (s_dump_bytecode) {
+            for (auto& block : unit.basic_blocks)
+                block.dump();
+        }
 
         if (s_run_bytecode) {
             JS::Bytecode::Interpreter bytecode_interpreter(interpreter.global_object());
-            bytecode_interpreter.run(*block);
+            bytecode_interpreter.run(unit);
         }
 
         return true;