[maglev] Add Int32 compare ops

Add an implementation of compare ops which, like binary ops,
speculatively reads integers (but still returns a tagged true/false
value).

Bug: v8:7700
Change-Id: I38f0ba99f8f7af30c89d0b987e28483c9610463f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3657440
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80692}

src/maglev/maglev-graph-builder.cc

@@ -168,6 +168,12 @@ bool BinaryOperationHasInt32FastPath() {
     case Operation::kShiftLeft:
     case Operation::kShiftRight:
     case Operation::kShiftRightLogical:
+    case Operation::kEqual:
+    case Operation::kStrictEqual:
+    case Operation::kLessThan:
+    case Operation::kLessThanOrEqual:
+    case Operation::kGreaterThan:
+    case Operation::kGreaterThanOrEqual:
       return true;
     default:
       return false;
@@ -204,6 +210,14 @@ bool BinaryOperationHasFloat64FastPath() {
   V(ShiftRight, Int32ShiftRight, 0)         \
   V(ShiftRightLogical, Int32ShiftRightLogical, 0)
 
+#define MAP_COMPARE_OPERATION_TO_INT32_NODE(V) \
+  V(Equal, Int32Equal)                         \
+  V(StrictEqual, Int32StrictEqual)             \
+  V(LessThan, Int32LessThan)                   \
+  V(LessThanOrEqual, Int32LessThanOrEqual)     \
+  V(GreaterThan, Int32GreaterThan)             \
+  V(GreaterThanOrEqual, Int32GreaterThanOrEqual)
+
 // MAP_OPERATION_TO_FLOAT64_NODE are tuples with the following format:
 // (Operation name, Float64 operation node).
 #define MAP_OPERATION_TO_FLOAT64_NODE(V) \
@@ -233,6 +247,11 @@ ValueNode* MaglevGraphBuilder::AddNewInt32BinaryOperationNode(
   case Operation::k##op:       \
     return AddNewNode<OpNode>(inputs);
     MAP_OPERATION_TO_INT32_NODE(CASE)
+#undef CASE
+#define CASE(op, OpNode) \
+  case Operation::k##op: \
+    return AddNewNode<OpNode>(inputs);
+    MAP_COMPARE_OPERATION_TO_INT32_NODE(CASE)
 #undef CASE
     default:
       UNREACHABLE();
@@ -409,11 +428,12 @@ void MaglevGraphBuilder::VisitCompareOperation() {
       if (BinaryOperationHasFloat64FastPath<kOperation>()) {
         BuildFloat64BinaryOperationNode<kOperation>();
         return;
-      } else if (BinaryOperationHasInt32FastPath<kOperation>()) {
-        // Fall back to int32 fast path if there is one (this will be the case
-        // for operations that deal with bits rather than numbers).
-        BuildInt32BinaryOperationNode<kOperation>();
-        return;
+        // } else if (BinaryOperationHasInt32FastPath<kOperation>()) {
+        //   // Fall back to int32 fast path if there is one (this will be the
+        //   case
+        //   // for operations that deal with bits rather than numbers).
+        //   BuildInt32BinaryOperationNode<kOperation>();
+        //   return;
       }
       break;
     default:

src/maglev/maglev-graph-verifier.h

@@ -152,6 +152,13 @@ class MaglevGraphVerifier {
       case Opcode::kInt32ShiftLeft:
       case Opcode::kInt32ShiftRight:
       case Opcode::kInt32ShiftRightLogical:
+      case Opcode::kInt32Equal:
+      case Opcode::kInt32StrictEqual:
+      case Opcode::kInt32LessThan:
+      case Opcode::kInt32LessThanOrEqual:
+      case Opcode::kInt32GreaterThan:
+      case Opcode::kInt32GreaterThanOrEqual:
+      case Opcode::kBranchIfInt32Compare:
         DCHECK_EQ(node->input_count(), 2);
         CheckValueInputIs(node, 0, ValueRepresentation::kInt32);
         CheckValueInputIs(node, 1, ValueRepresentation::kInt32);

src/maglev/maglev-interpreter-frame-state.h

@@ -517,10 +517,11 @@ class MergePointInterpreterFrameState {
                       ValueNode* unmerged, int merge_offset) {
     Phi* result = merged->TryCast<Phi>();
     if (result == nullptr || result->merge_offset() != merge_offset) {
-      DCHECK_EQ(merged, (unmerged->Is<CheckedSmiUntag>() ||
-                         unmerged->Is<CheckedFloat64Unbox>())
-                            ? unmerged->input(0).node()
-                            : unmerged);
+      if (merged != unmerged) {
+        DCHECK(unmerged->Is<CheckedSmiUntag>() ||
+               unmerged->Is<CheckedFloat64Unbox>());
+        DCHECK_EQ(merged, unmerged->input(0).node());
+      }
       return;
     }
     DCHECK_EQ(result->owner(), owner);

src/maglev/maglev-ir.cc

@@ -1130,6 +1130,74 @@ void Int32ShiftRightLogical::GenerateCode(MaglevCodeGenState* code_gen_state,
   __ shrl_cl(left);
 }
 
+namespace {
+
+constexpr Condition Int32ConditionFor(Operation operation) {
+  switch (operation) {
+    case Operation::kEqual:
+    case Operation::kStrictEqual:
+      return equal;
+    case Operation::kLessThan:
+      return less;
+    case Operation::kLessThanOrEqual:
+      return less_equal;
+    case Operation::kGreaterThan:
+      return greater;
+    case Operation::kGreaterThanOrEqual:
+      return greater_equal;
+    default:
+      UNREACHABLE();
+  }
+}
+
+}  // namespace
+
+template <class Derived, Operation kOperation>
+void Int32CompareNode<Derived, kOperation>::AllocateVreg(
+    MaglevVregAllocationState* vreg_state, const ProcessingState& state) {
+  UseRegister(left_input());
+  UseRegister(right_input());
+  DefineAsRegister(vreg_state, this);
+}
+
+template <class Derived, Operation kOperation>
+void Int32CompareNode<Derived, kOperation>::GenerateCode(
+    MaglevCodeGenState* code_gen_state, const ProcessingState& state) {
+  Register left = ToRegister(left_input());
+  Register right = ToRegister(right_input());
+  Register result = ToRegister(this->result());
+  Label is_true, end;
+  __ cmpl(left, right);
+  // TODO(leszeks): Investigate using cmov here.
+  __ j(Int32ConditionFor(kOperation), &is_true);
+  // TODO(leszeks): Investigate loading existing materialisations of roots here,
+  // if available.
+  __ LoadRoot(result, RootIndex::kFalseValue);
+  __ jmp(&end);
+  {
+    __ bind(&is_true);
+    __ LoadRoot(result, RootIndex::kTrueValue);
+  }
+  __ bind(&end);
+}
+
+#define DEF_OPERATION(Name)                                      \
+  void Name::AllocateVreg(MaglevVregAllocationState* vreg_state, \
+                          const ProcessingState& state) {        \
+    Base::AllocateVreg(vreg_state, state);                       \
+  }                                                              \
+  void Name::GenerateCode(MaglevCodeGenState* code_gen_state,    \
+                          const ProcessingState& state) {        \
+    Base::GenerateCode(code_gen_state, state);                   \
+  }
+DEF_OPERATION(Int32Equal)
+DEF_OPERATION(Int32StrictEqual)
+DEF_OPERATION(Int32LessThan)
+DEF_OPERATION(Int32LessThanOrEqual)
+DEF_OPERATION(Int32GreaterThan)
+DEF_OPERATION(Int32GreaterThanOrEqual)
+#undef DEF_OPERATION
+
 void Float64Add::AllocateVreg(MaglevVregAllocationState* vreg_state,
                               const ProcessingState& state) {
   UseRegister(left_input());
@@ -1578,9 +1646,9 @@ void BranchIfTrue::GenerateCode(MaglevCodeGenState* code_gen_state,
   }
 }
 
-void BranchIfCompare::AllocateVreg(MaglevVregAllocationState* vreg_state,
+void BranchIfInt32Compare::AllocateVreg(MaglevVregAllocationState* vreg_state,
                                         const ProcessingState& state) {}
-void BranchIfCompare::GenerateCode(MaglevCodeGenState* code_gen_state,
+void BranchIfInt32Compare::GenerateCode(MaglevCodeGenState* code_gen_state,
                                         const ProcessingState& state) {
   USE(operation_);
   UNREACHABLE();

src/maglev/maglev-ir.h

@@ -82,12 +82,12 @@ class CompactInterpreterFrameState;
   /*V(Int32NegateWithOverflow) */      \
   /*V(Int32IncrementWithOverflow)*/    \
   /*V(Int32DecrementWithOverflow)*/    \
-  /*V(Int32Equal)*/                    \
-  /*V(Int32StrictEqual) */             \
-  /*V(Int32LessThan)*/                 \
-  /*V(Int32LessThanOrEqual) */         \
-  /*V(Int32GreaterThan)*/              \
-  /*V(Int32GreaterThanOrEqual)*/
+  V(Int32Equal)                        \
+  V(Int32StrictEqual)                  \
+  V(Int32LessThan)                     \
+  V(Int32LessThanOrEqual)              \
+  V(Int32GreaterThan)                  \
+  V(Int32GreaterThanOrEqual)
 
 #define FLOAT64_OPERATIONS_NODE_LIST(V) \
   V(Float64Add)                         \
@@ -100,9 +100,9 @@ class CompactInterpreterFrameState;
   /*V(Float64Increment)*/               \
   /*V(Float64Decrement)*/               \
   /*V(Float64Equal)*/                   \
-  /*V(Float64StrictEqual) */            \
+  /*V(Float64StrictEqual)*/             \
   /*V(Float64LessThan)*/                \
-  /*V(Float64LessThanOrEqual) */        \
+  /*V(Float64LessThanOrEqual)*/         \
   /*V(Float64GreaterThan)*/             \
   /*V(Float64GreaterThanOrEqual)*/
 
@@ -149,7 +149,8 @@ class CompactInterpreterFrameState;
 
 #define CONDITIONAL_CONTROL_NODE_LIST(V) \
   V(BranchIfTrue)                        \
-  V(BranchIfToBooleanTrue)
+  V(BranchIfToBooleanTrue)               \
+  V(BranchIfInt32Compare)
 
 #define UNCONDITIONAL_CONTROL_NODE_LIST(V) \
   V(Jump)                                  \
@@ -1182,6 +1183,45 @@ DEF_INT32_BINARY_NODE(ShiftRightLogical)
 // DEF_INT32_UNARY_WITH_OVERFLOW_NODE(Increment)
 // DEF_INT32_UNARY_WITH_OVERFLOW_NODE(Decrement)
 
+template <class Derived, Operation kOperation>
+class Int32CompareNode : public FixedInputValueNodeT<2, Derived> {
+  using Base = FixedInputValueNodeT<2, Derived>;
+
+ public:
+  static constexpr int kLeftIndex = 0;
+  static constexpr int kRightIndex = 1;
+  Input& left_input() { return Node::input(kLeftIndex); }
+  Input& right_input() { return Node::input(kRightIndex); }
+
+ protected:
+  explicit Int32CompareNode(uint32_t bitfield) : Base(bitfield) {}
+
+  void AllocateVreg(MaglevVregAllocationState*, const ProcessingState&);
+  void GenerateCode(MaglevCodeGenState*, const ProcessingState&);
+  void PrintParams(std::ostream&, MaglevGraphLabeller*) const {}
+};
+
+#define DEF_OPERATION_NODE(Name, Super, OpName)                            \
+  class Name : public Super<Name, Operation::k##OpName> {                  \
+    using Base = Super<Name, Operation::k##OpName>;                        \
+                                                                           \
+   public:                                                                 \
+    explicit Name(uint32_t bitfield) : Base(bitfield) {}                   \
+    void AllocateVreg(MaglevVregAllocationState*, const ProcessingState&); \
+    void GenerateCode(MaglevCodeGenState*, const ProcessingState&);        \
+    void PrintParams(std::ostream&, MaglevGraphLabeller*) const {}         \
+  };
+
+#define DEF_INT32_COMPARE_NODE(Name) \
+  DEF_OPERATION_NODE(Int32##Name, Int32CompareNode, Name)
+DEF_INT32_COMPARE_NODE(Equal)
+DEF_INT32_COMPARE_NODE(StrictEqual)
+DEF_INT32_COMPARE_NODE(LessThan)
+DEF_INT32_COMPARE_NODE(LessThanOrEqual)
+DEF_INT32_COMPARE_NODE(GreaterThan)
+DEF_INT32_COMPARE_NODE(GreaterThanOrEqual)
+#undef DEF_INT32_COMPARE_NODE
+
 #undef DEF_OPERATION_NODE
 
 template <class Derived, Operation kOperation>
@@ -1221,6 +1261,12 @@ DEF_FLOAT64_BINARY_NODE(Multiply)
 DEF_FLOAT64_BINARY_NODE(Divide)
 // DEF_FLOAT64_BINARY_NODE(Modulus)
 // DEF_FLOAT64_BINARY_NODE(Exponentiate)
+// DEF_FLOAT64_BINARY_NODE(Equal)
+// DEF_FLOAT64_BINARY_NODE(StrictEqual)
+// DEF_FLOAT64_BINARY_NODE(LessThan)
+// DEF_FLOAT64_BINARY_NODE(LessThanOrEqual)
+// DEF_FLOAT64_BINARY_NODE(GreaterThan)
+// DEF_FLOAT64_BINARY_NODE(GreaterThanOrEqual)
 #undef DEF_FLOAT64_BINARY_NODE
 
 class CheckedSmiTag : public FixedInputValueNodeT<1, CheckedSmiTag> {
@@ -2214,9 +2260,9 @@ class BranchIfToBooleanTrue
   void PrintParams(std::ostream&, MaglevGraphLabeller*) const {}
 };
 
-class BranchIfCompare
-    : public ConditionalControlNodeT<2, BranchIfToBooleanTrue> {
-  using Base = ConditionalControlNodeT<2, BranchIfToBooleanTrue>;
+class BranchIfInt32Compare
+    : public ConditionalControlNodeT<2, BranchIfInt32Compare> {
+  using Base = ConditionalControlNodeT<2, BranchIfInt32Compare>;
 
  public:
   static constexpr int kLeftIndex = 0;
@@ -2224,7 +2270,7 @@ class BranchIfCompare
   Input& left_input() { return NodeBase::input(kLeftIndex); }
   Input& right_input() { return NodeBase::input(kRightIndex); }
 
-  explicit BranchIfCompare(uint32_t bitfield, Operation operation,
+  explicit BranchIfInt32Compare(uint32_t bitfield, Operation operation,
                                 BasicBlockRef* if_true_refs,
                                 BasicBlockRef* if_false_refs)
       : Base(bitfield, if_true_refs, if_false_refs), operation_(operation) {}