Revert "LibJS/Bytecode: Add peephole optimization pass and fuse compare+jump"

This reverts commit 4438ec481cddf01ee3a5193bb8c81f4f579f6660.

Fixes #23480.

Userland/Libraries/LibJS/Bytecode/BasicBlock.h

@@ -82,15 +82,15 @@ public:
         op->set_source_record({ start_offset, end_offset });
     }
 
+private:
+    explicit BasicBlock(String name);
+
     void terminate(size_t slot_offset)
     {
         m_terminated = true;
         m_terminator_offset = slot_offset;
     }
 
-private:
-    explicit BasicBlock(String name);
-
     Vector<u8> m_buffer;
     BasicBlock const* m_handler { nullptr };
     BasicBlock const* m_finalizer { nullptr };

Userland/Libraries/LibJS/Bytecode/Instruction.h

@@ -69,14 +69,6 @@
     O(IteratorToArray)                 \
     O(Jump)                            \
     O(JumpIf)                          \
-    O(JumpGreaterThan)                 \
-    O(JumpGreaterThanEquals)           \
-    O(JumpLessThan)                    \
-    O(JumpLessThanEquals)              \
-    O(JumpLooselyEquals)               \
-    O(JumpLooselyInequals)             \
-    O(JumpStrictlyEquals)              \
-    O(JumpStrictlyInequals)            \
     O(JumpNullish)                     \
     O(JumpUndefined)                   \
     O(LeaveLexicalEnvironment)         \

Userland/Libraries/LibJS/Bytecode/Interpreter.cpp

@@ -360,33 +360,6 @@ void Interpreter::run_bytecode()
                 else
                     m_current_block = &static_cast<Op::JumpIf const&>(instruction).false_target()->block();
                 goto start;
-
-#define JS_HANDLE_FUSABLE_BINARY_JUMP(PreOp, int32_operator, slow_case) \
-    case Instruction::Type::Jump##PreOp: {                              \
-        auto& jump = static_cast<Op::Jump##PreOp const&>(instruction);  \
-        auto lhs = get(jump.lhs());                                     \
-        auto rhs = get(jump.rhs());                                     \
-        bool condition = false;                                         \
-        if (lhs.is_int32() && rhs.is_int32()) {                         \
-            condition = lhs.as_i32() int32_operator rhs.as_i32();       \
-        } else {                                                        \
-            auto condition_or_error = slow_case(vm(), lhs, rhs);        \
-            if (condition_or_error.is_error()) {                        \
-                result = condition_or_error.release_error();            \
-                break;                                                  \
-            }                                                           \
-            condition = condition_or_error.value().to_boolean();        \
-        }                                                               \
-                                                                        \
-        if (condition)                                                  \
-            m_current_block = &jump.true_target()->block();             \
-        else                                                            \
-            m_current_block = &jump.false_target()->block();            \
-        goto start;                                                     \
-    }
-
-                JS_ENUMERATE_FUSABLE_BINARY_OPS(JS_HANDLE_FUSABLE_BINARY_JUMP)
-
             case Instruction::Type::JumpNullish:
                 if (get(static_cast<Op::JumpNullish const&>(instruction).condition()).is_nullish())
                     m_current_block = &static_cast<Op::Jump const&>(instruction).true_target()->block();
@@ -608,7 +581,6 @@ static PassManager& optimization_pipeline()
         pm->add<Passes::UnifySameBlocks>();
         pm->add<Passes::GenerateCFG>();
         pm->add<Passes::MergeBlocks>();
-        pm->add<Passes::Peephole>();
         pm->add<Passes::GenerateCFG>();
         pm->add<Passes::PlaceBlocks>();
         return pm;
@@ -651,6 +623,7 @@ Variant<NonnullOwnPtr<CallFrame>, CallFrame*> Interpreter::pop_call_frame()
     m_current_call_frame = m_call_frames.is_empty() ? Span<Value> {} : this->call_frame().registers();
     return frame;
 }
+
 }
 
 namespace JS::Bytecode {
@@ -1282,20 +1255,6 @@ ThrowCompletionOr<void> JumpIf::execute_impl(Bytecode::Interpreter&) const
     __builtin_unreachable();
 }
 
-#define JS_DEFINE_FUSABLE_BINARY_OP(PreOp, ...)                                                                  \
-    ThrowCompletionOr<void> Jump##PreOp::execute_impl(Bytecode::Interpreter&) const { __builtin_unreachable(); } \
-                                                                                                                 \
-    ByteString Jump##PreOp::to_byte_string_impl(Bytecode::Executable const& executable) const                    \
-    {                                                                                                            \
-        return ByteString::formatted("Jump" #PreOp " {}, {}, \033[32mtrue\033[0m:{} \033[32mfalse\033[0m:{}",    \
-            format_operand("lhs"sv, m_lhs, executable),                                                          \
-            format_operand("rhs"sv, m_rhs, executable),                                                          \
-            *m_true_target,                                                                                      \
-            *m_false_target);                                                                                    \
-    }
-
-JS_ENUMERATE_FUSABLE_BINARY_OPS(JS_DEFINE_FUSABLE_BINARY_OP)
-
 ThrowCompletionOr<void> JumpUndefined::execute_impl(Bytecode::Interpreter&) const
 {
     // Handled in the interpreter loop.

Userland/Libraries/LibJS/Bytecode/Op.h

@@ -1123,40 +1123,6 @@ private:
     Operand m_condition;
 };
 
-// NOTE: The raw operator is used for comparing two Int32 values.
-#define JS_ENUMERATE_FUSABLE_BINARY_OPS(X)        \
-    X(GreaterThan, >, greater_than)               \
-    X(GreaterThanEquals, >=, greater_than_equals) \
-    X(LessThan, <, less_than)                     \
-    X(LessThanEquals, <=, less_than_equals)       \
-    X(LooselyEquals, ==, loosely_equals)          \
-    X(LooselyInequals, !=, loosely_inequals)      \
-    X(StrictlyEquals, ==, strict_equals)          \
-    X(StrictlyInequals, !=, strict_inequals)
-
-#define JS_DECLARE_FUSED_JUMP(PreOp, ...)                                                     \
-    class Jump##PreOp final : public Jump {                                                   \
-    public:                                                                                   \
-        explicit Jump##PreOp(Operand lhs, Operand rhs, Label true_target, Label false_target) \
-            : Jump(Type::Jump##PreOp, move(true_target), move(false_target), sizeof(*this))   \
-            , m_lhs(lhs)                                                                      \
-            , m_rhs(rhs)                                                                      \
-        {                                                                                     \
-        }                                                                                     \
-        ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const;                   \
-        ByteString to_byte_string_impl(Bytecode::Executable const&) const;                    \
-                                                                                              \
-        Operand lhs() const { return m_lhs; }                                                 \
-        Operand rhs() const { return m_rhs; }                                                 \
-                                                                                              \
-    private:                                                                                  \
-        Operand m_lhs;                                                                        \
-        Operand m_rhs;                                                                        \
-    };
-
-JS_ENUMERATE_FUSABLE_BINARY_OPS(JS_DECLARE_FUSED_JUMP)
-#undef JS_DECLARE_FUSED_JUMP
-
 class JumpNullish final : public Jump {
 public:
     explicit JumpNullish(Operand condition, Label true_target, Label false_target)

Userland/Libraries/LibJS/Bytecode/Pass/GenerateCFG.cpp

@@ -63,13 +63,6 @@ static void generate_cfg_for_block(BasicBlock const& current_block, PassPipeline
             enter_label(true_target, current_block);
             return;
         }
-
-#define JS_ENUMERATE_FUSABLE_BINARY_OP(PreOp, ...) \
-    case Jump##PreOp:
-
-            JS_ENUMERATE_FUSABLE_BINARY_OPS(JS_ENUMERATE_FUSABLE_BINARY_OP)
-#undef JS_ENUMERATE_FUSABLE_BINARY_OP
-
         case JumpIf:
         case JumpNullish:
         case JumpUndefined: {

Userland/Libraries/LibJS/Bytecode/Pass/Peephole.cpp

@@ -1,90 +0,0 @@
-/*
- * Copyright (c) 2024, Andreas Kling <kling@serenityos.org>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#include <LibJS/Bytecode/PassManager.h>
-
-namespace JS::Bytecode::Passes {
-
-void Peephole::perform(PassPipelineExecutable& executable)
-{
-    started();
-
-    // Fuse compare-followed-by-jump into a single compare-and-jump
-    // This is a very common pattern in bytecode, and it's nice to have it as a single instruction
-    // For example, LessThan + JumpIf => JumpLessThan
-
-    HashMap<BasicBlock const*, BasicBlock const*> replacement_blocks;
-    Vector<NonnullOwnPtr<BasicBlock>> replaced_blocks;
-
-    for (size_t i = 0; i < executable.executable.basic_blocks.size(); ++i) {
-        auto& block = executable.executable.basic_blocks[i];
-        auto new_block = BasicBlock::create(block->name());
-        if (block->handler())
-            new_block->set_handler(*block->handler());
-        if (block->finalizer())
-            new_block->set_finalizer(*block->finalizer());
-        replacement_blocks.set(block.ptr(), new_block.ptr());
-
-        InstructionStreamIterator it { block->instruction_stream() };
-        while (!it.at_end()) {
-            auto const& instruction = *it;
-            ++it;
-
-            if (!it.at_end()) {
-                auto const& next_instruction = *it;
-                if (next_instruction.type() == Instruction::Type::JumpIf) {
-                    auto const& jump = static_cast<Op::JumpIf const&>(next_instruction);
-
-#define DO_FUSE_JUMP(PreOp, ...)                                          \
-    if (instruction.type() == Instruction::Type::PreOp) {                 \
-        auto const& compare = static_cast<Op::PreOp const&>(instruction); \
-        VERIFY(jump.condition() == compare.dst());                        \
-        new_block->append<Op::Jump##PreOp>(                               \
-            compare.source_record().source_start_offset,                  \
-            compare.source_record().source_end_offset,                    \
-            compare.lhs(),                                                \
-            compare.rhs(),                                                \
-            *jump.true_target(),                                          \
-            *jump.false_target());                                        \
-        ++it;                                                             \
-        VERIFY(it.at_end());                                              \
-        continue;                                                         \
-    }
-                    JS_ENUMERATE_FUSABLE_BINARY_OPS(DO_FUSE_JUMP)
-                }
-            }
-
-            auto slot_offset = new_block->size();
-            new_block->grow(instruction.length());
-            memcpy(new_block->data() + slot_offset, &instruction, instruction.length());
-            if (instruction.is_terminator())
-                new_block->terminate(slot_offset);
-        }
-        replaced_blocks.append(move(executable.executable.basic_blocks[i]));
-        executable.executable.basic_blocks[i] = move(new_block);
-    }
-
-    auto update_block_references = [&](BasicBlock const& original, BasicBlock const& replacement) {
-        for (auto& block : executable.executable.basic_blocks) {
-            InstructionStreamIterator it { block->instruction_stream() };
-            if (block->handler() == &original)
-                block->set_handler(replacement);
-            if (block->finalizer() == &original)
-                block->set_finalizer(replacement);
-            while (!it.at_end()) {
-                auto const& instruction = *it;
-                ++it;
-                const_cast<Instruction&>(instruction).replace_references(original, replacement);
-            }
-        }
-    };
-    for (auto& entry : replacement_blocks)
-        update_block_references(*entry.key, *entry.value);
-
-    finished();
-}
-
-}

Userland/Libraries/LibJS/Bytecode/PassManager.h

@@ -107,15 +107,6 @@ private:
     virtual void perform(PassPipelineExecutable&) override;
 };
 
-class Peephole : public Pass {
-public:
-    Peephole() = default;
-    ~Peephole() override = default;
-
-private:
-    virtual void perform(PassPipelineExecutable&) override;
-};
-
 class DumpCFG : public Pass {
 public:
     DumpCFG(FILE* file)

Userland/Libraries/LibJS/CMakeLists.txt

@@ -12,7 +12,6 @@ set(SOURCES
     Bytecode/Pass/DumpCFG.cpp
     Bytecode/Pass/GenerateCFG.cpp
     Bytecode/Pass/MergeBlocks.cpp
-    Bytecode/Pass/Peephole.cpp
     Bytecode/Pass/PlaceBlocks.cpp
     Bytecode/Pass/UnifySameBlocks.cpp
     Bytecode/RegexTable.cpp