[maglev] Add Box/UnboxNumbers and Float64Add nodes

- For simplicity we call a builtin when allocating a number.
- Elision of boxing/unboxing nodes will be done in a followup CL.

Bug: v8:7700
Change-Id: Iec4422d84c6597d3369ab512a1662adb0f077c98
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3602514Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Auto-Submit: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80178}

src/builtins/builtins-definitions.h

@@ -240,6 +240,8 @@ namespace internal {
   TFC(AllocateInOldGeneration, Allocate)                                       \
   TFC(AllocateRegularInOldGeneration, Allocate)                                \
                                                                                \
+  TFC(NewHeapNumber, NewHeapNumber)                                            \
+                                                                               \
   /* TurboFan support builtins */                                              \
   TFS(CopyFastSmiOrObjectElements, kObject)                                    \
   TFC(GrowFastDoubleElements, GrowArrayElements)                               \

src/builtins/builtins-internal-gen.cc

@@ -1080,6 +1080,11 @@ TF_BUILTIN(AdaptorWithBuiltinExitFrame, CodeStubAssembler) {
                new_target);         // additional stack argument 4
 }
 
+TF_BUILTIN(NewHeapNumber, CodeStubAssembler) {
+  auto val = UncheckedParameter<Float64T>(Descriptor::kValue);
+  Return(ChangeFloat64ToTagged(val));
+}
+
 TF_BUILTIN(AllocateInYoungGeneration, CodeStubAssembler) {
   auto requested_size = UncheckedParameter<IntPtrT>(Descriptor::kRequestedSize);
   CSA_CHECK(this, IsValidPositiveSmi(requested_size));

src/codegen/ia32/interface-descriptors-ia32-inl.h

@@ -310,6 +310,13 @@ constexpr auto WasmFloat64ToNumberDescriptor::registers() {
   return RegisterArray(ecx);
 }
 
+// static
+constexpr auto NewHeapNumberDescriptor::registers() {
+  // Work around using eax, whose register code is 0, and leads to the FP
+  // parameter being passed via xmm0, which is not allocatable on ia32.
+  return RegisterArray(ecx);
+}
+
 }  // namespace internal
 }  // namespace v8
 

src/codegen/interface-descriptors-inl.h

@@ -189,6 +189,14 @@ StaticCallInterfaceDescriptor<DerivedDescriptor>::GetRegisterParameter(int i) {
   return DerivedDescriptor::registers()[i];
 }
 
+// static
+template <typename DerivedDescriptor>
+constexpr DoubleRegister
+StaticCallInterfaceDescriptor<DerivedDescriptor>::GetDoubleRegisterParameter(
+    int i) {
+  return DoubleRegister::from_code(DerivedDescriptor::registers()[i].code());
+}
+
 // static
 constexpr Register FastNewObjectDescriptor::TargetRegister() {
   return kJSFunctionRegister;

src/codegen/interface-descriptors.h

@@ -102,6 +102,7 @@ namespace internal {
   V(LoadWithReceiverBaseline)                        \
   V(LoadWithVector)                                  \
   V(LookupBaseline)                                  \
+  V(NewHeapNumber)                                   \
   V(NoContext)                                       \
   V(ResumeGenerator)                                 \
   V(ResumeGeneratorBaseline)                         \
@@ -480,6 +481,10 @@ class StaticCallInterfaceDescriptor : public CallInterfaceDescriptor {
   static constexpr inline Register* GetRegisterData();
   static constexpr inline Register GetRegisterParameter(int i);
 
+  // Interface descriptors don't really support double registers.
+  // This reinterprets the i-th register as a double with the same code.
+  static constexpr inline DoubleRegister GetDoubleRegisterParameter(int i);
+
   explicit StaticCallInterfaceDescriptor(CallDescriptors::Key key)
       : CallInterfaceDescriptor(key) {}
 
@@ -714,6 +719,20 @@ class AllocateDescriptor
   static constexpr auto registers();
 };
 
+class NewHeapNumberDescriptor
+    : public StaticCallInterfaceDescriptor<NewHeapNumberDescriptor> {
+ public:
+  DEFINE_PARAMETERS_NO_CONTEXT(kValue)
+  DEFINE_RESULT_AND_PARAMETER_TYPES(MachineType::TaggedPointer(),  // Result
+                                    MachineType::Float64())        // kValue
+  DECLARE_DESCRIPTOR(NewHeapNumberDescriptor)
+
+#if V8_TARGET_ARCH_IA32
+  // We need a custom descriptor on ia32 to avoid using xmm0.
+  static constexpr inline auto registers();
+#endif
+};
+
 // This descriptor defines the JavaScript calling convention that can be used
 // by stubs: target, new.target, argc and context are passed in registers while
 // receiver and the rest of the JS arguments are passed on the stack.

src/maglev/maglev-code-gen-state.h

@@ -142,10 +142,19 @@ inline Register ToRegister(const compiler::InstructionOperand& operand) {
   return compiler::AllocatedOperand::cast(operand).GetRegister();
 }
 
+inline DoubleRegister ToDoubleRegister(
+    const compiler::InstructionOperand& operand) {
+  return compiler::AllocatedOperand::cast(operand).GetDoubleRegister();
+}
+
 inline Register ToRegister(const ValueLocation& location) {
   return ToRegister(location.operand());
 }
 
+inline DoubleRegister ToDoubleRegister(const ValueLocation& location) {
+  return ToDoubleRegister(location.operand());
+}
+
 inline void MaglevCodeGenState::DefineSafepointStackSlots(
     SafepointTableBuilder::Safepoint& safepoint) const {
   for (int stack_slot = 0; stack_slot < tagged_slots_; stack_slot++) {

src/maglev/maglev-graph-builder.cc

@@ -207,6 +207,22 @@ void MaglevGraphBuilder::VisitBinaryOperation() {
           SetAccumulator(AddNewNode<Int32AddWithOverflow>({left, right}));
           return;
         }
+
+      } else if (hint == BinaryOperationHint::kNumber) {
+        ValueNode *left, *right;
+        if (IsRegisterEqualToAccumulator(0)) {
+          left = right =
+              AddNewNode<CheckedFloat64Unbox>({LoadRegisterTagged(0)});
+        } else {
+          left = AddNewNode<CheckedFloat64Unbox>({LoadRegisterTagged(0)});
+          right = AddNewNode<CheckedFloat64Unbox>({GetAccumulatorTagged()});
+        }
+
+        if (kOperation == Operation::kAdd) {
+          ValueNode* result = AddNewNode<Float64Add>({left, right});
+          SetAccumulator(AddNewNode<Float64Box>({result}));
+          return;
+        }
       }
     }
   }

src/maglev/maglev-graph-verifier.h

@@ -88,14 +88,18 @@ class MaglevGraphVerifier {
       case Opcode::kBranchIfTrue:
       case Opcode::kBranchIfToBooleanTrue:
       case Opcode::kReturn:
-        // Generic tagged unary operations.
+      case Opcode::kCheckedFloat64Unbox:
         DCHECK_EQ(node->input_count(), 1);
         CheckValueInputIs(node, 0, ValueRepresentation::kTagged);
         break;
       case Opcode::kCheckedSmiTag:
-        // Untagged unary operations.
+        DCHECK_EQ(node->input_count(), 1);
         CheckValueInputIs(node, 0, ValueRepresentation::kInt32);
         break;
+      case Opcode::kFloat64Box:
+        DCHECK_EQ(node->input_count(), 1);
+        CheckValueInputIs(node, 0, ValueRepresentation::kFloat64);
+        break;
       case Opcode::kGenericAdd:
       case Opcode::kGenericSubtract:
       case Opcode::kGenericMultiply:
@@ -119,16 +123,20 @@ class MaglevGraphVerifier {
       // TODO(victorgomes): Can we check that first input is an Object?
       case Opcode::kStoreField:
       case Opcode::kLoadNamedGeneric:
-        // Generic tagged binary operations.
         DCHECK_EQ(node->input_count(), 2);
         CheckValueInputIs(node, 0, ValueRepresentation::kTagged);
         CheckValueInputIs(node, 1, ValueRepresentation::kTagged);
         break;
       case Opcode::kInt32AddWithOverflow:
-        // Untagged binary operations.
+        DCHECK_EQ(node->input_count(), 2);
         CheckValueInputIs(node, 0, ValueRepresentation::kInt32);
         CheckValueInputIs(node, 1, ValueRepresentation::kInt32);
         break;
+      case Opcode::kFloat64Add:
+        DCHECK_EQ(node->input_count(), 2);
+        CheckValueInputIs(node, 0, ValueRepresentation::kFloat64);
+        CheckValueInputIs(node, 1, ValueRepresentation::kFloat64);
+        break;
       case Opcode::kCall:
       case Opcode::kPhi:
         // All inputs should be tagged.

src/maglev/maglev-ir.cc

@@ -85,6 +85,10 @@ void UseFixed(Input& input, Register reg) {
   input.SetUnallocated(compiler::UnallocatedOperand::FIXED_REGISTER, reg.code(),
                        GetVirtualRegister(input.node()));
 }
+void UseFixed(Input& input, DoubleRegister reg) {
+  input.SetUnallocated(compiler::UnallocatedOperand::FIXED_FP_REGISTER,
+                       reg.code(), GetVirtualRegister(input.node()));
+}
 
 // ---
 // Code gen helpers.
@@ -748,6 +752,58 @@ void Int32AddWithOverflow::GenerateCode(MaglevCodeGenState* code_gen_state,
   EmitEagerDeoptIf(overflow, code_gen_state, this);
 }
 
+void Float64Box::AllocateVreg(MaglevVregAllocationState* vreg_state,
+                              const ProcessingState& state) {
+  using D = NewHeapNumberDescriptor;
+  UseFixed(input(), D::GetDoubleRegisterParameter(D::kValue));
+  DefineAsFixed(vreg_state, this, kReturnRegister0);
+}
+void Float64Box::GenerateCode(MaglevCodeGenState* code_gen_state,
+                              const ProcessingState& state) {
+  // TODO(victorgomes): Inline heap number allocation.
+  __ CallBuiltin(Builtin::kNewHeapNumber);
+}
+
+void CheckedFloat64Unbox::AllocateVreg(MaglevVregAllocationState* vreg_state,
+                                       const ProcessingState& state) {
+  UseRegister(input());
+  DefineAsRegister(vreg_state, this);
+}
+void CheckedFloat64Unbox::GenerateCode(MaglevCodeGenState* code_gen_state,
+                                       const ProcessingState& state) {
+  Register value = ToRegister(input());
+  Label is_not_smi, done;
+  // Check if Smi.
+  __ testb(value, Immediate(1));
+  __ j(not_zero, &is_not_smi);
+  // If Smi, convert to Float64.
+  __ sarl(value, Immediate(1));
+  __ Cvtlsi2sd(ToDoubleRegister(result()), value);
+  __ jmp(&done);
+  __ bind(&is_not_smi);
+  // Check if HeapNumber, deopt otherwise.
+  __ CompareRoot(FieldOperand(value, HeapObject::kMapOffset),
+                 RootIndex::kHeapNumberMap);
+  EmitEagerDeoptIf(not_equal, code_gen_state, this);
+  __ Movsd(ToDoubleRegister(result()),
+           FieldOperand(value, HeapNumber::kValueOffset));
+  __ bind(&done);
+}
+
+void Float64Add::AllocateVreg(MaglevVregAllocationState* vreg_state,
+                              const ProcessingState& state) {
+  UseRegister(left_input());
+  UseRegister(right_input());
+  DefineSameAsFirst(vreg_state, this);
+}
+
+void Float64Add::GenerateCode(MaglevCodeGenState* code_gen_state,
+                              const ProcessingState& state) {
+  DoubleRegister left = ToDoubleRegister(left_input());
+  DoubleRegister right = ToDoubleRegister(right_input());
+  __ Addsd(left, right);
+}
+
 void Phi::AllocateVreg(MaglevVregAllocationState* vreg_state,
                        const ProcessingState& state) {
   // Phi inputs are processed in the post-process, once loop phis' inputs'

src/maglev/maglev-ir.h

@@ -80,6 +80,9 @@ class CompactInterpreterFrameState;
   V(CheckedSmiUntag)       \
   V(Int32AddWithOverflow)  \
   V(Int32Constant)         \
+  V(Float64Box)            \
+  V(CheckedFloat64Unbox)   \
+  V(Float64Add)            \
   GENERIC_OPERATIONS_NODE_LIST(V)
 
 #define NODE_LIST(V) \
@@ -828,6 +831,12 @@ class ValueNode : public Node {
 
   compiler::AllocatedOperand allocation() const {
     if (has_register()) {
+      if (use_double_register()) {
+        return compiler::AllocatedOperand(
+            compiler::LocationOperand::REGISTER,
+            MachineRepresentation::kFloat64,
+            double_registers_with_result_.first().code());
+      }
       return compiler::AllocatedOperand(compiler::LocationOperand::REGISTER,
                                         GetMachineRepresentation(),
                                         FirstRegisterCode());
@@ -1106,6 +1115,56 @@ class Int32AddWithOverflow
   void PrintParams(std::ostream&, MaglevGraphLabeller*) const {}
 };
 
+class Float64Box : public FixedInputValueNodeT<1, Float64Box> {
+  using Base = FixedInputValueNodeT<1, Float64Box>;
+
+ public:
+  explicit Float64Box(uint32_t bitfield) : Base(bitfield) {}
+
+  static constexpr OpProperties kProperties = OpProperties::Call();
+
+  Input& input() { return Node::input(0); }
+
+  void AllocateVreg(MaglevVregAllocationState*, const ProcessingState&);
+  void GenerateCode(MaglevCodeGenState*, const ProcessingState&);
+  void PrintParams(std::ostream&, MaglevGraphLabeller*) const {}
+};
+
+class CheckedFloat64Unbox
+    : public FixedInputValueNodeT<1, CheckedFloat64Unbox> {
+  using Base = FixedInputValueNodeT<1, CheckedFloat64Unbox>;
+
+ public:
+  explicit CheckedFloat64Unbox(uint32_t bitfield) : Base(bitfield) {}
+
+  static constexpr OpProperties kProperties =
+      OpProperties::EagerDeopt() | OpProperties::Float64();
+
+  Input& input() { return Node::input(0); }
+
+  void AllocateVreg(MaglevVregAllocationState*, const ProcessingState&);
+  void GenerateCode(MaglevCodeGenState*, const ProcessingState&);
+  void PrintParams(std::ostream&, MaglevGraphLabeller*) const {}
+};
+
+class Float64Add : public FixedInputValueNodeT<2, Float64Add> {
+  using Base = FixedInputValueNodeT<2, Float64Add>;
+
+ public:
+  explicit Float64Add(uint32_t bitfield) : Base(bitfield) {}
+
+  static constexpr OpProperties kProperties = OpProperties::Float64();
+
+  static constexpr int kLeftIndex = 0;
+  static constexpr int kRightIndex = 1;
+  Input& left_input() { return Node::input(kLeftIndex); }
+  Input& right_input() { return Node::input(kRightIndex); }
+
+  void AllocateVreg(MaglevVregAllocationState*, const ProcessingState&);
+  void GenerateCode(MaglevCodeGenState*, const ProcessingState&);
+  void PrintParams(std::ostream&, MaglevGraphLabeller*) const {}
+};
+
 class InitialValue : public FixedInputValueNodeT<0, InitialValue> {
   using Base = FixedInputValueNodeT<0, InitialValue>;
 

test/mjsunit/maglev/add-number.js

+// Copyright 2022 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax --maglev --no-stress-opt
+
+(function() {
+  function add(x, y) {
+    return x + y;
+  }
+
+  %PrepareFunctionForOptimization(add);
+  assertEquals(4.2, add(2.1, 2.1));
+
+  %OptimizeMaglevOnNextCall(add);
+  assertEquals(4.2, add(2.1, 2.1));
+  assertTrue(isMaglevved(add));
+
+  // We don't deopt if we use smis.
+  assertEquals(42, add(22, 20));
+  assertTrue(isMaglevved(add));
+
+  // We deopt if not a number.
+  assertEquals("42", add("4", "2"));
+  assertFalse(isMaglevved(add));
+
+  // TODO(victorgomes): Fix deopt when we have a float,
+  // i.e., add(4, "2") will create a float with number 4
+  // and correctly deopt, but the state is bogus.
+})();