[maglev] Split GapMove in two classes, and some cleanup

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

src/maglev/maglev-basic-block.h

@@ -33,7 +33,7 @@ class BasicBlock {
     if (has_phi()) return phis()->first()->id();
     if (!nodes_.is_empty()) {
       for (const Node* node : nodes_) {
-        if (node->Is<GapMove>()) continue;
+        if (IsGapMoveNode(node->opcode())) continue;
         return node->id();
       }
     }

src/maglev/maglev-graph-verifier.h

@@ -61,39 +61,40 @@ class MaglevGraphVerifier {
   void Process(NodeBase* node, const ProcessingState& state) {
     switch (node->opcode()) {
       case Opcode::kConstant:
-      case Opcode::kSmiConstant:
-      case Opcode::kInt32Constant:
+      case Opcode::kConstantGapMove:
+      case Opcode::kCreateEmptyArrayLiteral:
+      case Opcode::kDeopt:
       case Opcode::kFloat64Constant:
-      case Opcode::kRootConstant:
-      case Opcode::kInitialValue:
-      case Opcode::kRegisterInput:
       case Opcode::kGapMove:
-      case Opcode::kDeopt:
+      case Opcode::kInitialValue:
+      case Opcode::kInt32Constant:
       case Opcode::kJump:
+      case Opcode::kJumpFromInlined:
       case Opcode::kJumpLoop:
       case Opcode::kJumpToInlined:
-      case Opcode::kJumpFromInlined:
-      case Opcode::kCreateEmptyArrayLiteral:
+      case Opcode::kRegisterInput:
+      case Opcode::kRootConstant:
+      case Opcode::kSmiConstant:
         // No input.
         DCHECK_EQ(node->input_count(), 0);
         break;
-      case Opcode::kGenericNegate:
-      case Opcode::kGenericIncrement:
-      case Opcode::kGenericDecrement:
       case Opcode::kCheckedSmiUntag:
       case Opcode::kGenericBitwiseNot:
-      case Opcode::kLoadTaggedField:
+      case Opcode::kGenericDecrement:
+      case Opcode::kGenericIncrement:
+      case Opcode::kGenericNegate:
       case Opcode::kLoadDoubleField:
       case Opcode::kLoadGlobal:
+      case Opcode::kLoadTaggedField:
       // TODO(victorgomes): Can we check that the input is actually a map?
       case Opcode::kCheckMaps:
       // TODO(victorgomes): Can we check that the input is Boolean?
-      case Opcode::kBranchIfTrue:
       case Opcode::kBranchIfToBooleanTrue:
-      case Opcode::kReturn:
+      case Opcode::kBranchIfTrue:
       case Opcode::kCheckedFloat64Unbox:
       case Opcode::kCreateObjectLiteral:
       case Opcode::kCreateShallowObjectLiteral:
+      case Opcode::kReturn:
         DCHECK_EQ(node->input_count(), 1);
         CheckValueInputIs(node, 0, ValueRepresentation::kTagged);
         break;
@@ -107,25 +108,25 @@ class MaglevGraphVerifier {
         CheckValueInputIs(node, 0, ValueRepresentation::kFloat64);
         break;
       case Opcode::kGenericAdd:
-      case Opcode::kGenericSubtract:
-      case Opcode::kGenericMultiply:
-      case Opcode::kGenericDivide:
-      case Opcode::kGenericModulus:
-      case Opcode::kGenericExponentiate:
       case Opcode::kGenericBitwiseAnd:
       case Opcode::kGenericBitwiseOr:
       case Opcode::kGenericBitwiseXor:
+      case Opcode::kGenericDivide:
+      case Opcode::kGenericExponentiate:
+      case Opcode::kGenericModulus:
+      case Opcode::kGenericMultiply:
       case Opcode::kGenericShiftLeft:
       case Opcode::kGenericShiftRight:
       case Opcode::kGenericShiftRightLogical:
+      case Opcode::kGenericSubtract:
       // TODO(victorgomes): Can we use the fact that these nodes return a
       // Boolean?
       case Opcode::kGenericEqual:
-      case Opcode::kGenericStrictEqual:
-      case Opcode::kGenericLessThan:
-      case Opcode::kGenericLessThanOrEqual:
       case Opcode::kGenericGreaterThan:
       case Opcode::kGenericGreaterThanOrEqual:
+      case Opcode::kGenericLessThan:
+      case Opcode::kGenericLessThanOrEqual:
+      case Opcode::kGenericStrictEqual:
       // TODO(victorgomes): Can we check that first input is an Object?
       case Opcode::kStoreField:
       case Opcode::kLoadNamedGeneric:

src/maglev/maglev-ir.cc

@@ -399,18 +399,6 @@ DeoptInfo::DeoptInfo(Zone* zone, const MaglevCompilationUnit& compilation_unit,
 // ---
 // Nodes
 // ---
-void ValueNode::LoadToRegister(MaglevCodeGenState* code_gen_state,
-                               compiler::AllocatedOperand op) {
-  switch (opcode()) {
-#define V(Name)         \
-  case Opcode::k##Name: \
-    return this->Cast<Name>()->DoLoadToRegister(code_gen_state, op);
-    VALUE_NODE_LIST(V)
-#undef V
-    default:
-      UNREACHABLE();
-  }
-}
 void ValueNode::LoadToRegister(MaglevCodeGenState* code_gen_state,
                                Register reg) {
   switch (opcode()) {
@@ -434,7 +422,7 @@ Handle<Object> ValueNode::Reify(Isolate* isolate) {
 #define V(Name)         \
   case Opcode::k##Name: \
     return this->Cast<Name>()->DoReify(isolate);
-    VALUE_NODE_LIST(V)
+    CONSTANT_VALUE_NODE_LIST(V)
 #undef V
     default:
       UNREACHABLE();
@@ -446,8 +434,7 @@ void ValueNode::SetNoSpillOrHint() {
 #ifdef DEBUG
   state_ = kSpillOrHint;
 #endif  // DEBUG
-  if (Is<Constant>() || Is<SmiConstant>() || Is<RootConstant>() ||
-      Is<Int32Constant>() || Is<Float64Constant>()) {
+  if (IsConstantNode(opcode())) {
     spill_or_hint_ = compiler::ConstantOperand(
         compiler::UnallocatedOperand::cast(result().operand())
             .virtual_register());
@@ -462,10 +449,6 @@ void SmiConstant::AllocateVreg(MaglevVregAllocationState* vreg_state,
 }
 void SmiConstant::GenerateCode(MaglevCodeGenState* code_gen_state,
                                const ProcessingState& state) {}
-void SmiConstant::DoLoadToRegister(MaglevCodeGenState* code_gen_state,
-                                   compiler::AllocatedOperand op) {
-  DoLoadToRegister(code_gen_state, op.GetRegister());
-}
 Handle<Object> SmiConstant::DoReify(Isolate* isolate) {
   return handle(value_, isolate);
 }
@@ -484,10 +467,6 @@ void Float64Constant::AllocateVreg(MaglevVregAllocationState* vreg_state,
 }
 void Float64Constant::GenerateCode(MaglevCodeGenState* code_gen_state,
                                    const ProcessingState& state) {}
-void Float64Constant::DoLoadToRegister(MaglevCodeGenState* code_gen_state,
-                                       compiler::AllocatedOperand op) {
-  DoLoadToRegister(code_gen_state, op.GetDoubleRegister());
-}
 Handle<Object> Float64Constant::DoReify(Isolate* isolate) {
   return isolate->factory()->NewNumber(value_);
 }
@@ -506,10 +485,6 @@ void Constant::AllocateVreg(MaglevVregAllocationState* vreg_state,
 }
 void Constant::GenerateCode(MaglevCodeGenState* code_gen_state,
                             const ProcessingState& state) {}
-void Constant::DoLoadToRegister(MaglevCodeGenState* code_gen_state,
-                                compiler::AllocatedOperand op) {
-  DoLoadToRegister(code_gen_state, op.GetRegister());
-}
 void Constant::DoLoadToRegister(MaglevCodeGenState* code_gen_state,
                                 Register reg) {
   __ Move(reg, object_.object());
@@ -588,10 +563,6 @@ void RootConstant::AllocateVreg(MaglevVregAllocationState* vreg_state,
 }
 void RootConstant::GenerateCode(MaglevCodeGenState* code_gen_state,
                                 const ProcessingState& state) {}
-void RootConstant::DoLoadToRegister(MaglevCodeGenState* code_gen_state,
-                                    compiler::AllocatedOperand op) {
-  DoLoadToRegister(code_gen_state, op.GetRegister());
-}
 void RootConstant::DoLoadToRegister(MaglevCodeGenState* code_gen_state,
                                     Register reg) {
   __ LoadRoot(reg, index());
@@ -839,9 +810,7 @@ void GapMove::AllocateVreg(MaglevVregAllocationState* vreg_state,
 }
 void GapMove::GenerateCode(MaglevCodeGenState* code_gen_state,
                            const ProcessingState& state) {
-  if (source().IsConstant()) {
-    node_->LoadToRegister(code_gen_state, target());
-  } else if (source().IsRegister()) {
+  if (source().IsRegister()) {
     Register source_reg = ToRegister(source());
     if (target().IsAnyRegister()) {
       DCHECK(target().IsRegister());
@@ -875,6 +844,46 @@ void GapMove::PrintParams(std::ostream& os,
                           MaglevGraphLabeller* graph_labeller) const {
   os << "(" << source() << " → " << target() << ")";
 }
+void ConstantGapMove::AllocateVreg(MaglevVregAllocationState* vreg_state,
+                                   const ProcessingState& state) {
+  UNREACHABLE();
+}
+
+namespace {
+template <typename T>
+struct GetRegister;
+template <>
+struct GetRegister<Register> {
+  static Register Get(compiler::AllocatedOperand target) {
+    return target.GetRegister();
+  }
+};
+template <>
+struct GetRegister<DoubleRegister> {
+  static DoubleRegister Get(compiler::AllocatedOperand target) {
+    return target.GetDoubleRegister();
+  }
+};
+};  // namespace
+void ConstantGapMove::GenerateCode(MaglevCodeGenState* code_gen_state,
+                                   const ProcessingState& state) {
+  switch (node_->opcode()) {
+#define CASE(Name)                                \
+  case Opcode::k##Name:                           \
+    return node_->Cast<Name>()->DoLoadToRegister( \
+        code_gen_state, GetRegister<Name::OutputRegister>::Get(target()));
+    CONSTANT_VALUE_NODE_LIST(CASE)
+#undef CASE
+    default:
+      UNREACHABLE();
+  }
+}
+void ConstantGapMove::PrintParams(std::ostream& os,
+                                  MaglevGraphLabeller* graph_labeller) const {
+  os << "(";
+  graph_labeller->PrintNodeLabel(os, node_);
+  os << " → " << target() << ")";
+}
 
 namespace {
 
@@ -978,10 +987,6 @@ void Int32Constant::AllocateVreg(MaglevVregAllocationState* vreg_state,
 }
 void Int32Constant::GenerateCode(MaglevCodeGenState* code_gen_state,
                                  const ProcessingState& state) {}
-void Int32Constant::DoLoadToRegister(MaglevCodeGenState* code_gen_state,
-                                     compiler::AllocatedOperand op) {
-  DoLoadToRegister(code_gen_state, op.GetRegister());
-}
 void Int32Constant::DoLoadToRegister(MaglevCodeGenState* code_gen_state,
                                      Register reg) {
   __ Move(reg, Immediate(value()));

src/maglev/maglev-ir.h

@@ -65,9 +65,15 @@ class CompactInterpreterFrameState;
   V(GenericGreaterThan)                 \
   V(GenericGreaterThanOrEqual)
 
+#define CONSTANT_VALUE_NODE_LIST(V) \
+  V(Constant)                       \
+  V(Float64Constant)                \
+  V(Int32Constant)                  \
+  V(RootConstant)                   \
+  V(SmiConstant)
+
 #define VALUE_NODE_LIST(V)      \
   V(Call)                       \
-  V(Constant)                   \
   V(Construct)                  \
   V(CreateEmptyArrayLiteral)    \
   V(CreateObjectLiteral)        \
@@ -80,23 +86,24 @@ class CompactInterpreterFrameState;
   V(SetNamedGeneric)            \
   V(Phi)                        \
   V(RegisterInput)              \
-  V(RootConstant)               \
-  V(SmiConstant)                \
   V(CheckedSmiTag)              \
   V(CheckedSmiUntag)            \
   V(Int32AddWithOverflow)       \
-  V(Int32Constant)              \
-  V(Float64Constant)            \
   V(ChangeInt32ToFloat64)       \
   V(Float64Box)                 \
   V(CheckedFloat64Unbox)        \
   V(Float64Add)                 \
+  CONSTANT_VALUE_NODE_LIST(V)   \
   GENERIC_OPERATIONS_NODE_LIST(V)
 
-#define NODE_LIST(V) \
-  V(CheckMaps)       \
-  V(GapMove)         \
-  V(StoreField)      \
+#define GAP_MOVE_NODE_LIST(V) \
+  V(ConstantGapMove)          \
+  V(GapMove)
+
+#define NODE_LIST(V)    \
+  V(CheckMaps)          \
+  V(StoreField)         \
+  GAP_MOVE_NODE_LIST(V) \
   VALUE_NODE_LIST(V)
 
 #define CONDITIONAL_CONTROL_NODE_LIST(V) \
@@ -139,6 +146,14 @@ static constexpr Opcode kFirstValueNodeOpcode =
     std::min({VALUE_NODE_LIST(V) kLastOpcode});
 static constexpr Opcode kLastValueNodeOpcode =
     std::max({VALUE_NODE_LIST(V) kFirstOpcode});
+static constexpr Opcode kFirstConstantNodeOpcode =
+    std::min({CONSTANT_VALUE_NODE_LIST(V) kLastOpcode});
+static constexpr Opcode kLastConstantNodeOpcode =
+    std::max({CONSTANT_VALUE_NODE_LIST(V) kFirstOpcode});
+static constexpr Opcode kFirstGapMoveNodeOpcode =
+    std::min({GAP_MOVE_NODE_LIST(V) kLastOpcode});
+static constexpr Opcode kLastGapMoveNodeOpcode =
+    std::max({GAP_MOVE_NODE_LIST(V) kFirstOpcode});
 
 static constexpr Opcode kFirstNodeOpcode = std::min({NODE_LIST(V) kLastOpcode});
 static constexpr Opcode kLastNodeOpcode = std::max({NODE_LIST(V) kFirstOpcode});
@@ -162,6 +177,13 @@ static constexpr Opcode kLastControlNodeOpcode =
 constexpr bool IsValueNode(Opcode opcode) {
   return kFirstValueNodeOpcode <= opcode && opcode <= kLastValueNodeOpcode;
 }
+constexpr bool IsConstantNode(Opcode opcode) {
+  return kFirstConstantNodeOpcode <= opcode &&
+         opcode <= kLastConstantNodeOpcode;
+}
+constexpr bool IsGapMoveNode(Opcode opcode) {
+  return kFirstGapMoveNodeOpcode <= opcode && opcode <= kLastGapMoveNodeOpcode;
+}
 constexpr bool IsConditionalControlNode(Opcode opcode) {
   return kFirstConditionalControlNodeOpcode <= opcode &&
          opcode <= kLastConditionalControlNodeOpcode;
@@ -712,7 +734,6 @@ class ValueNode : public Node {
   void SetNoSpillOrHint();
 
   /* For constants only. */
-  void LoadToRegister(MaglevCodeGenState*, compiler::AllocatedOperand);
   void LoadToRegister(MaglevCodeGenState*, Register);
   Handle<Object> Reify(Isolate* isolate);
 
@@ -724,11 +745,7 @@ class ValueNode : public Node {
       DCHECK(!is_loadable());
     }
 #endif  // DEBUG
-    DCHECK(!Is<Constant>());
-    DCHECK(!Is<SmiConstant>());
-    DCHECK(!Is<RootConstant>());
-    DCHECK(!Is<Int32Constant>());
-    DCHECK(!Is<Float64Constant>());
+    DCHECK(!IsConstantNode(opcode()));
     DCHECK(operand.IsAnyStackSlot());
     spill_or_hint_ = operand;
     DCHECK(spill_or_hint_.IsAnyStackSlot());
@@ -849,11 +866,7 @@ class ValueNode : public Node {
     return registers_with_result_.first().code();
   }
 
-  void DoLoadToRegister(MaglevCodeGenState*, compiler::AllocatedOperand) {
-    UNREACHABLE();
-  }
   void DoLoadToRegister(MaglevCodeGenState*, Register);
-  Handle<Object> DoReify(Isolate* isolate) { UNREACHABLE(); }
 
   // Rename for better pairing with `end_id`.
   NodeIdT start_id() const { return id(); }
@@ -1080,8 +1093,7 @@ class Int32Constant : public FixedInputValueNodeT<0, Int32Constant> {
   void GenerateCode(MaglevCodeGenState*, const ProcessingState&);
   void PrintParams(std::ostream&, MaglevGraphLabeller*) const;
 
-  void DoLoadToRegister(MaglevCodeGenState*, compiler::AllocatedOperand);
-  void DoLoadToRegister(MaglevCodeGenState*, Register);
+  void DoLoadToRegister(MaglevCodeGenState*, OutputRegister);
   Handle<Object> DoReify(Isolate* isolate);
 
  private:
@@ -1105,9 +1117,8 @@ class Float64Constant : public FixedInputValueNodeT<0, Float64Constant> {
   void GenerateCode(MaglevCodeGenState*, const ProcessingState&);
   void PrintParams(std::ostream&, MaglevGraphLabeller*) const;
 
-  void DoLoadToRegister(MaglevCodeGenState*, compiler::AllocatedOperand);
   void DoLoadToRegister(MaglevCodeGenState*, Register) { UNREACHABLE(); }
-  void DoLoadToRegister(MaglevCodeGenState*, DoubleRegister);
+  void DoLoadToRegister(MaglevCodeGenState*, OutputRegister);
   Handle<Object> DoReify(Isolate* isolate);
 
  private:
@@ -1238,6 +1249,8 @@ class SmiConstant : public FixedInputValueNodeT<0, SmiConstant> {
   using Base = FixedInputValueNodeT<0, SmiConstant>;
 
  public:
+  using OutputRegister = Register;
+
   explicit SmiConstant(uint32_t bitfield, Smi value)
       : Base(bitfield), value_(value) {}
 
@@ -1247,8 +1260,7 @@ class SmiConstant : public FixedInputValueNodeT<0, SmiConstant> {
   void GenerateCode(MaglevCodeGenState*, const ProcessingState&);
   void PrintParams(std::ostream&, MaglevGraphLabeller*) const;
 
-  void DoLoadToRegister(MaglevCodeGenState*, compiler::AllocatedOperand);
-  void DoLoadToRegister(MaglevCodeGenState*, Register);
+  void DoLoadToRegister(MaglevCodeGenState*, OutputRegister);
   Handle<Object> DoReify(Isolate* isolate);
 
  private:
@@ -1259,6 +1271,8 @@ class Constant : public FixedInputValueNodeT<0, Constant> {
   using Base = FixedInputValueNodeT<0, Constant>;
 
  public:
+  using OutputRegister = Register;
+
   explicit Constant(uint32_t bitfield, const compiler::HeapObjectRef& object)
       : Base(bitfield), object_(object) {}
 
@@ -1266,8 +1280,7 @@ class Constant : public FixedInputValueNodeT<0, Constant> {
   void GenerateCode(MaglevCodeGenState*, const ProcessingState&);
   void PrintParams(std::ostream&, MaglevGraphLabeller*) const;
 
-  void DoLoadToRegister(MaglevCodeGenState*, compiler::AllocatedOperand);
-  void DoLoadToRegister(MaglevCodeGenState*, Register);
+  void DoLoadToRegister(MaglevCodeGenState*, OutputRegister);
   Handle<Object> DoReify(Isolate* isolate);
 
  private:
@@ -1278,6 +1291,8 @@ class RootConstant : public FixedInputValueNodeT<0, RootConstant> {
   using Base = FixedInputValueNodeT<0, RootConstant>;
 
  public:
+  using OutputRegister = Register;
+
   explicit RootConstant(uint32_t bitfield, RootIndex index)
       : Base(bitfield), index_(index) {}
 
@@ -1287,8 +1302,7 @@ class RootConstant : public FixedInputValueNodeT<0, RootConstant> {
   void GenerateCode(MaglevCodeGenState*, const ProcessingState&);
   void PrintParams(std::ostream&, MaglevGraphLabeller*) const;
 
-  void DoLoadToRegister(MaglevCodeGenState*, compiler::AllocatedOperand);
-  void DoLoadToRegister(MaglevCodeGenState*, Register);
+  void DoLoadToRegister(MaglevCodeGenState*, OutputRegister);
   Handle<Object> DoReify(Isolate* isolate);
 
  private:
@@ -1555,12 +1569,30 @@ class GapMove : public FixedInputNodeT<0, GapMove> {
   using Base = FixedInputNodeT<0, GapMove>;
 
  public:
-  GapMove(uint32_t bitfield, ValueNode* node,
-          compiler::InstructionOperand source,
+  GapMove(uint32_t bitfield, compiler::AllocatedOperand source,
           compiler::AllocatedOperand target)
-      : Base(bitfield), node_(node), source_(source), target_(target) {}
+      : Base(bitfield), source_(source), target_(target) {}
+
+  compiler::AllocatedOperand source() const { return source_; }
+  compiler::AllocatedOperand target() const { return target_; }
+
+  void AllocateVreg(MaglevVregAllocationState*, const ProcessingState&);
+  void GenerateCode(MaglevCodeGenState*, const ProcessingState&);
+  void PrintParams(std::ostream&, MaglevGraphLabeller*) const;
+
+ private:
+  compiler::AllocatedOperand source_;
+  compiler::AllocatedOperand target_;
+};
+
+class ConstantGapMove : public FixedInputNodeT<0, ConstantGapMove> {
+  using Base = FixedInputNodeT<0, ConstantGapMove>;
+
+ public:
+  ConstantGapMove(uint32_t bitfield, ValueNode* node,
+                  compiler::AllocatedOperand target)
+      : Base(bitfield), node_(node), target_(target) {}
 
-  compiler::InstructionOperand source() const { return source_; }
   compiler::AllocatedOperand target() const { return target_; }
   ValueNode* node() const { return node_; }
 

src/maglev/maglev-regalloc.cc

@@ -496,9 +496,7 @@ void StraightForwardRegisterAllocator::AllocateNodeResult(ValueNode* node) {
     }
 
     case compiler::UnallocatedOperand::NONE:
-      DCHECK(node->Is<Constant>() || node->Is<RootConstant>() ||
-             node->Is<SmiConstant>() || node->Is<Int32Constant>() ||
-             node->Is<Float64Constant>());
+      DCHECK(IsConstantNode(node->opcode()));
       break;
 
     case compiler::UnallocatedOperand::MUST_HAVE_SLOT:
@@ -669,8 +667,16 @@ void StraightForwardRegisterAllocator::TryAllocateToInput(Phi* phi) {
 void StraightForwardRegisterAllocator::AddMoveBeforeCurrentNode(
     ValueNode* node, compiler::InstructionOperand source,
     compiler::AllocatedOperand target) {
-  GapMove* gap_move =
-      Node::New<GapMove>(compilation_info_->zone(), {}, node, source, target);
+  Node* gap_move;
+  DCHECK_EQ(source.IsConstant(), IsConstantNode(node->opcode()));
+  if (source.IsConstant()) {
+    gap_move =
+        Node::New<ConstantGapMove>(compilation_info_->zone(), {}, node, target);
+  } else {
+    gap_move =
+        Node::New<GapMove>(compilation_info_->zone(), {},
+                           compiler::AllocatedOperand::cast(source), target);
+  }
   if (compilation_info_->has_graph_labeller()) {
     graph_labeller()->RegisterNode(gap_move);
   }