From c2d3d9d1d4f33e2a646b5604878e31c256ce7b5b Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Sun, 5 May 2024 11:52:14 +0200
Subject: [PATCH] LibJS/Bytecode: Make each Jump instruction inherit
 Instruction directly

Before this change, all JumpFoo instructions inherited from Jump, which
forced the unconditional Jump to have an unusued "false target" member.
Also, labels were unnecessarily wrapped in Optional<>.

By defining each jump instruction separately, they all shrink in size,
and all ambiguity is removed.
---
 .../Libraries/LibJS/Bytecode/Interpreter.cpp  | 51 +++++------
 Userland/Libraries/LibJS/Bytecode/Op.h        | 87 ++++++++++---------
 2 files changed, 73 insertions(+), 65 deletions(-)

diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp
index 9298f86e61e..6f7fa4dd75d 100644
--- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp
+++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp
@@ -332,26 +332,32 @@ void Interpreter::run_bytecode()
                 accumulator = get(static_cast<Op::End const&>(instruction).value());
                 return;
             case Instruction::Type::Jump:
-                m_current_block = &static_cast<Op::Jump const&>(instruction).true_target()->block();
+                m_current_block = &static_cast<Op::Jump const&>(instruction).target().block();
                 goto start;
-            case Instruction::Type::JumpIf:
-                if (get(static_cast<Op::JumpIf const&>(instruction).condition()).to_boolean())
-                    m_current_block = &static_cast<Op::JumpIf const&>(instruction).true_target()->block();
+            case Instruction::Type::JumpIf: {
+                auto& jump = static_cast<Op::JumpIf const&>(instruction);
+                if (get(jump.condition()).to_boolean())
+                    m_current_block = &jump.true_target().block();
                 else
-                    m_current_block = &static_cast<Op::JumpIf const&>(instruction).false_target()->block();
+                    m_current_block = &jump.false_target().block();
                 goto start;
-            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();
+            }
+            case Instruction::Type::JumpNullish: {
+                auto& jump = static_cast<Op::JumpNullish const&>(instruction);
+                if (get(jump.condition()).is_nullish())
+                    m_current_block = &jump.true_target().block();
                 else
-                    m_current_block = &static_cast<Op::Jump const&>(instruction).false_target()->block();
+                    m_current_block = &jump.false_target().block();
                 goto start;
-            case Instruction::Type::JumpUndefined:
-                if (get(static_cast<Op::JumpUndefined const&>(instruction).condition()).is_undefined())
-                    m_current_block = &static_cast<Op::Jump const&>(instruction).true_target()->block();
+            }
+            case Instruction::Type::JumpUndefined: {
+                auto& jump = static_cast<Op::JumpUndefined const&>(instruction);
+                if (get(jump.condition()).is_undefined())
+                    m_current_block = &jump.true_target().block();
                 else
-                    m_current_block = &static_cast<Op::Jump const&>(instruction).false_target()->block();
+                    m_current_block = &jump.false_target().block();
                 goto start;
+            }
             case Instruction::Type::EnterUnwindContext:
                 enter_unwind_context();
                 m_current_block = &static_cast<Op::EnterUnwindContext const&>(instruction).entry_point().block();
@@ -1869,36 +1875,31 @@ ByteString DeleteByIdWithThis::to_byte_string_impl(Bytecode::Executable const& e
 
 ByteString Jump::to_byte_string_impl(Bytecode::Executable const&) const
 {
-    if (m_true_target.has_value())
-        return ByteString::formatted("Jump {}", *m_true_target);
-    return ByteString::formatted("Jump <empty>");
+    return ByteString::formatted("Jump {}", m_target);
 }
 
 ByteString JumpIf::to_byte_string_impl(Bytecode::Executable const& executable) const
 {
-    auto true_string = m_true_target.has_value() ? ByteString::formatted("{}", *m_true_target) : "<empty>";
-    auto false_string = m_false_target.has_value() ? ByteString::formatted("{}", *m_false_target) : "<empty>";
     return ByteString::formatted("JumpIf {}, \033[32mtrue\033[0m:{} \033[32mfalse\033[0m:{}",
         format_operand("condition"sv, m_condition, executable),
-        true_string, false_string);
+        m_true_target,
+        m_false_target);
 }
 
 ByteString JumpNullish::to_byte_string_impl(Bytecode::Executable const& executable) const
 {
-    auto true_string = m_true_target.has_value() ? ByteString::formatted("{}", *m_true_target) : "<empty>";
-    auto false_string = m_false_target.has_value() ? ByteString::formatted("{}", *m_false_target) : "<empty>";
     return ByteString::formatted("JumpNullish {}, null:{} nonnull:{}",
         format_operand("condition"sv, m_condition, executable),
-        true_string, false_string);
+        m_true_target,
+        m_false_target);
 }
 
 ByteString JumpUndefined::to_byte_string_impl(Bytecode::Executable const& executable) const
 {
-    auto true_string = m_true_target.has_value() ? ByteString::formatted("{}", *m_true_target) : "<empty>";
-    auto false_string = m_false_target.has_value() ? ByteString::formatted("{}", *m_false_target) : "<empty>";
     return ByteString::formatted("JumpUndefined {}, undefined:{} defined:{}",
         format_operand("condition"sv, m_condition, executable),
-        true_string, false_string);
+        m_true_target,
+        m_false_target);
 }
 
 static StringView call_type_to_string(CallType type)
diff --git a/Userland/Libraries/LibJS/Bytecode/Op.h b/Userland/Libraries/LibJS/Bytecode/Op.h
index 6b3567f8fad..a2999cf4608 100644
--- a/Userland/Libraries/LibJS/Bytecode/Op.h
+++ b/Userland/Libraries/LibJS/Bytecode/Op.h
@@ -1057,64 +1057,59 @@ private:
     Operand m_property;
 };
 
-class Jump : public Instruction {
+class Jump final : public Instruction {
 public:
     constexpr static bool IsTerminator = true;
 
-    explicit Jump(Type type, Label taken_target, Optional<Label> nontaken_target = {})
-        : Instruction(type, sizeof(*this))
-        , m_true_target(move(taken_target))
-        , m_false_target(move(nontaken_target))
-    {
-    }
-
-    explicit Jump(Type type, Label taken_target, Label nontaken_target, size_t sizeof_self)
-        : Instruction(type, sizeof_self)
-        , m_true_target(move(taken_target))
-        , m_false_target(move(nontaken_target))
-    {
-    }
-
-    explicit Jump(Label taken_target, Optional<Label> nontaken_target = {})
+    explicit Jump(Label target)
         : Instruction(Type::Jump, sizeof(*this))
-        , m_true_target(move(taken_target))
-        , m_false_target(move(nontaken_target))
+        , m_target(target)
     {
     }
 
     ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const;
     ByteString to_byte_string_impl(Bytecode::Executable const&) const;
 
+    auto& target() const { return m_target; }
+
+protected:
+    Label m_target;
+};
+
+class JumpIf final : public Instruction {
+public:
+    constexpr static bool IsTerminator = true;
+
+    explicit JumpIf(Operand condition, Label true_target, Label false_target)
+        : Instruction(Type::JumpIf, sizeof(*this))
+        , m_condition(condition)
+        , m_true_target(true_target)
+        , m_false_target(false_target)
+    {
+    }
+
+    ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const;
+    ByteString to_byte_string_impl(Bytecode::Executable const&) const;
+
+    Operand condition() const { return m_condition; }
     auto& true_target() const { return m_true_target; }
     auto& false_target() const { return m_false_target; }
 
-protected:
-    Optional<Label> m_true_target;
-    Optional<Label> m_false_target;
-};
-
-class JumpIf final : public Jump {
-public:
-    explicit JumpIf(Operand condition, Label true_target, Label false_target)
-        : Jump(Type::JumpIf, move(true_target), move(false_target), sizeof(*this))
-        , m_condition(condition)
-    {
-    }
-
-    ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const;
-    ByteString to_byte_string_impl(Bytecode::Executable const&) const;
-
-    Operand condition() const { return m_condition; }
-
 private:
     Operand m_condition;
+    Label m_true_target;
+    Label m_false_target;
 };
 
-class JumpNullish final : public Jump {
+class JumpNullish final : public Instruction {
 public:
+    constexpr static bool IsTerminator = true;
+
     explicit JumpNullish(Operand condition, Label true_target, Label false_target)
-        : Jump(Type::JumpNullish, move(true_target), move(false_target), sizeof(*this))
+        : Instruction(Type::JumpNullish, sizeof(*this))
         , m_condition(condition)
+        , m_true_target(true_target)
+        , m_false_target(false_target)
     {
     }
 
@@ -1122,16 +1117,24 @@ public:
     ByteString to_byte_string_impl(Bytecode::Executable const&) const;
 
     Operand condition() const { return m_condition; }
+    auto& true_target() const { return m_true_target; }
+    auto& false_target() const { return m_false_target; }
 
 private:
     Operand m_condition;
+    Label m_true_target;
+    Label m_false_target;
 };
 
-class JumpUndefined final : public Jump {
+class JumpUndefined final : public Instruction {
 public:
+    constexpr static bool IsTerminator = true;
+
     explicit JumpUndefined(Operand condition, Label true_target, Label false_target)
-        : Jump(Type::JumpUndefined, move(true_target), move(false_target), sizeof(*this))
+        : Instruction(Type::JumpUndefined, sizeof(*this))
         , m_condition(condition)
+        , m_true_target(true_target)
+        , m_false_target(false_target)
     {
     }
 
@@ -1139,9 +1142,13 @@ public:
     ByteString to_byte_string_impl(Bytecode::Executable const&) const;
 
     Operand condition() const { return m_condition; }
+    auto& true_target() const { return m_true_target; }
+    auto& false_target() const { return m_false_target; }
 
 private:
     Operand m_condition;
+    Label m_true_target;
+    Label m_false_target;
 };
 
 enum class CallType {