[maglev] Float64 box/unbox elision

- Supports Float64 Add for SmiAdd bytecode
- Adds a Float64Constant and ChangeInt32ToFloat64 nodes
- Converts floats to tagged in Phi node inputs
- Fixes spill double representation
- Fixes materialisation during a deopt of a double in the stack

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

src/maglev/maglev-code-generator.cc

@@ -124,7 +124,7 @@ class MaglevCodeGeneratingNodeProcessor {
         compiler::AllocatedOperand source =
             compiler::AllocatedOperand::cast(value_node->result().operand());
         // We shouldn't spill nodes which already output to the stack.
-        if (!source.IsStackSlot()) {
+        if (!source.IsAnyStackSlot()) {
           if (FLAG_code_comments) __ RecordComment("--   Spill:");
           if (source.IsRegister()) {
             __ movq(code_gen_state_->GetStackSlot(value_node->spill_slot()),
@@ -502,7 +502,7 @@ class MaglevCodeGeneratorImpl final {
         translation_array_builder_.StoreInt32StackSlot(stack_slot);
         break;
       case ValueRepresentation::kFloat64:
-        translation_array_builder_.StoreInt64StackSlot(stack_slot);
+        translation_array_builder_.StoreDoubleStackSlot(stack_slot);
         break;
     }
   }

src/maglev/maglev-graph-builder.cc

@@ -267,19 +267,26 @@ void MaglevGraphBuilder::BuildInt32BinarySmiOperationNode() {
   SetAccumulator(AddNewInt32BinaryOperationNode<kOperation>({left, right}));
 }
 
+template <Operation kOperation>
+void MaglevGraphBuilder::BuildFloat64BinarySmiOperationNode() {
+  // TODO(v8:7700): Do constant folding.
+  ValueNode* left = GetAccumulatorFloat64();
+  double constant = static_cast<double>(iterator_.GetImmediateOperand(0));
+  ValueNode* right = AddNewNode<Float64Constant>({}, constant);
+  SetAccumulator(AddNewFloat64BinaryOperationNode<kOperation>({left, right}));
+}
+
 template <Operation kOperation>
 void MaglevGraphBuilder::BuildFloat64BinaryOperationNode() {
   // TODO(v8:7700): Do constant folding.
   ValueNode *left, *right;
   if (IsRegisterEqualToAccumulator(0)) {
-    left = right = AddNewNode<CheckedFloat64Unbox>({LoadRegisterTagged(0)});
+    left = right = LoadRegisterFloat64(0);
   } else {
-    left = AddNewNode<CheckedFloat64Unbox>({LoadRegisterTagged(0)});
-    right = AddNewNode<CheckedFloat64Unbox>({GetAccumulatorTagged()});
+    left = LoadRegisterFloat64(0);
+    right = GetAccumulatorFloat64();
   }
-  ValueNode* result =
-      AddNewFloat64BinaryOperationNode<kOperation>({left, right});
-  SetAccumulator(AddNewNode<Float64Box>({result}));
+  SetAccumulator(AddNewFloat64BinaryOperationNode<kOperation>({left, right}));
 }
 
 template <Operation kOperation>
@@ -315,6 +322,9 @@ void MaglevGraphBuilder::VisitBinarySmiOperation() {
       case BinaryOperationHint::kSignedSmall:
         BuildInt32BinarySmiOperationNode<kOperation>();
         return;
+      case BinaryOperationHint::kNumber:
+        BuildFloat64BinarySmiOperationNode<kOperation>();
+        return;
       default:
         // Fallback to generic node.
         break;

src/maglev/maglev-graph-builder.h

@@ -299,10 +299,18 @@ class MaglevGraphBuilder {
     current_interpreter_frame_.set(dst, current_interpreter_frame_.get(src));
   }
 
-  ValueNode* GetTaggedValue(interpreter::Register reg) {
-    // TODO(victorgomes): Add the representation (Tagged/Untagged) in the
+  template <typename NodeT>
+  ValueNode* AddNewConversionNode(interpreter::Register reg, ValueNode* node) {
+    // TODO(v8:7700): Use a canonical conversion node. Maybe like in Phi nodes
+    // where we always add a the conversion immediately after the ValueNode.
+    ValueNode* result = AddNewNode<NodeT>({node});
+    current_interpreter_frame_.set(reg, result);
+    return result;
+  }
+
+  ValueNode* GetTaggedValueHelper(interpreter::Register reg, ValueNode* value) {
+    // TODO(victorgomes): Consider adding the representation in the
     // InterpreterFrameState, so that we don't need to derefence a node.
-    ValueNode* value = current_interpreter_frame_.get(reg);
     switch (value->properties().value_representation()) {
       case ValueRepresentation::kTagged:
         return value;
@@ -310,40 +318,60 @@ class MaglevGraphBuilder {
         if (value->Is<CheckedSmiUntag>()) {
           return value->input(0).node();
         }
-        DCHECK(value->Is<Int32AddWithOverflow>() || value->Is<Int32Constant>());
-        ValueNode* tagged = AddNewNode<CheckedSmiTag>({value});
-        current_interpreter_frame_.set(reg, tagged);
-        return tagged;
+        return AddNewConversionNode<CheckedSmiTag>(reg, value);
       }
       case ValueRepresentation::kFloat64: {
         if (value->Is<CheckedFloat64Unbox>()) {
           return value->input(0).node();
         }
-        DCHECK(value->Is<LoadDoubleField>() || value->Is<Float64Add>());
-        ValueNode* tagged = AddNewNode<Float64Box>({value});
-        current_interpreter_frame_.set(reg, tagged);
-        return tagged;
+        if (value->Is<ChangeInt32ToFloat64>()) {
+          ValueNode* int32_value = value->input(0).node();
+          return GetTaggedValueHelper(reg, int32_value);
+        }
+        return AddNewConversionNode<Float64Box>(reg, value);
       }
     }
     UNREACHABLE();
   }
 
+  ValueNode* GetTaggedValue(interpreter::Register reg) {
+    ValueNode* value = current_interpreter_frame_.get(reg);
+    return GetTaggedValueHelper(reg, value);
+  }
+
   ValueNode* GetInt32(interpreter::Register reg) {
-    // TODO(victorgomes): Add the representation (Tagged/Untagged) in the
-    // InterpreterFrameState, so that we don't need to derefence a node.
-    // TODO(victorgomes): Support Float64.
     ValueNode* value = current_interpreter_frame_.get(reg);
-    if (value->properties().value_representation() ==
-        ValueRepresentation::kInt32) {
-      return value;
+    switch (value->properties().value_representation()) {
+      case ValueRepresentation::kTagged: {
+        if (value->Is<CheckedSmiTag>()) {
+          return value->input(0).node();
+        }
+        return AddNewConversionNode<CheckedSmiUntag>(reg, value);
+      }
+      case ValueRepresentation::kInt32:
+        return value;
+      case ValueRepresentation::kFloat64:
+        // We should not be able to request an Int32 from a Float64 input.
+        UNREACHABLE();
     }
-    if (value->Is<CheckedSmiTag>()) return value->input(0).node();
-    // Untag any other value.
-    DCHECK_EQ(value->properties().value_representation(),
-              ValueRepresentation::kTagged);
-    ValueNode* untagged = AddNewNode<CheckedSmiUntag>({value});
-    current_interpreter_frame_.set(reg, untagged);
-    return untagged;
+    UNREACHABLE();
+  }
+
+  ValueNode* GetFloat64(interpreter::Register reg) {
+    ValueNode* value = current_interpreter_frame_.get(reg);
+    switch (value->properties().value_representation()) {
+      case ValueRepresentation::kTagged: {
+        if (value->Is<Float64Box>()) {
+          return value->input(0).node();
+        }
+        return AddNewConversionNode<CheckedFloat64Unbox>(reg, value);
+      }
+      case ValueRepresentation::kInt32:
+        return AddNewConversionNode<ChangeInt32ToFloat64>(reg, value);
+      case ValueRepresentation::kFloat64:
+        return value;
+    }
+    UNREACHABLE();
   }
 
   ValueNode* GetAccumulatorTagged() {
@@ -354,6 +382,10 @@ class MaglevGraphBuilder {
     return GetInt32(interpreter::Register::virtual_accumulator());
   }
 
+  ValueNode* GetAccumulatorFloat64() {
+    return GetFloat64(interpreter::Register::virtual_accumulator());
+  }
+
   bool IsRegisterEqualToAccumulator(int operand_index) {
     interpreter::Register source = iterator_.GetRegisterOperand(operand_index);
     return current_interpreter_frame_.get(source) ==
@@ -368,6 +400,10 @@ class MaglevGraphBuilder {
     return GetInt32(iterator_.GetRegisterOperand(operand_index));
   }
 
+  ValueNode* LoadRegisterFloat64(int operand_index) {
+    return GetFloat64(iterator_.GetRegisterOperand(operand_index));
+  }
+
   template <typename NodeT>
   void SetAccumulator(NodeT* node) {
     // Accumulator stores are equivalent to stores to the virtual accumulator
@@ -529,6 +565,8 @@ class MaglevGraphBuilder {
   void BuildInt32BinarySmiOperationNode();
   template <Operation kOperation>
   void BuildFloat64BinaryOperationNode();
+  template <Operation kOperation>
+  void BuildFloat64BinarySmiOperationNode();
 
   template <Operation kOperation>
   void VisitUnaryOperation();

src/maglev/maglev-graph-verifier.h

@@ -63,6 +63,7 @@ class MaglevGraphVerifier {
       case Opcode::kConstant:
       case Opcode::kSmiConstant:
       case Opcode::kInt32Constant:
+      case Opcode::kFloat64Constant:
       case Opcode::kRootConstant:
       case Opcode::kInitialValue:
       case Opcode::kRegisterInput:
@@ -94,6 +95,7 @@ class MaglevGraphVerifier {
         CheckValueInputIs(node, 0, ValueRepresentation::kTagged);
         break;
       case Opcode::kCheckedSmiTag:
+      case Opcode::kChangeInt32ToFloat64:
         DCHECK_EQ(node->input_count(), 1);
         CheckValueInputIs(node, 0, ValueRepresentation::kInt32);
         break;

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

@@ -369,9 +369,8 @@ class MergePointInterpreterFrameState {
       const MaglevCompilationUnit& info,
       const MergePointInterpreterFrameState& state);
 
-  ValueNode* TagValue(MaglevCompilationUnit& compilation_unit,
-                      ValueNode* value) {
-    // TODO(victorgomes): Support Float64.
+  ValueNode* FromInt32ToTagged(MaglevCompilationUnit& compilation_unit,
+                               ValueNode* value) {
     DCHECK_EQ(value->properties().value_representation(),
               ValueRepresentation::kInt32);
     if (value->Is<CheckedSmiUntag>()) {
@@ -396,16 +395,42 @@ class MergePointInterpreterFrameState {
     return tagged;
   }
 
+  ValueNode* FromFloat64ToTagged(MaglevCompilationUnit& compilation_unit,
+                                 ValueNode* value) {
+    DCHECK_EQ(value->properties().value_representation(),
+              ValueRepresentation::kFloat64);
+    if (value->Is<CheckedFloat64Unbox>()) {
+      return value->input(0).node();
+    }
+    if (value->Is<ChangeInt32ToFloat64>()) {
+      return FromInt32ToTagged(compilation_unit, value->input(0).node());
+    }
+    // Check if the next Node in the block after value is its Float64Box
+    // version and reuse it.
+    if (value->NextNode()) {
+      Float64Box* tagged = value->NextNode()->TryCast<Float64Box>();
+      if (tagged != nullptr && value == tagged->input().node()) {
+        return tagged;
+      }
+    }
+    // Otherwise create a tagged version.
+    ValueNode* tagged = Node::New<Float64Box>(compilation_unit.zone(), {value});
+    Node::List::AddAfter(value, tagged);
+    compilation_unit.RegisterNodeInGraphLabeller(tagged);
+    return tagged;
+  }
+
+  // TODO(victorgomes): Consider refactor this function to share code with
+  // MaglevGraphBuilder::GetTagged.
   ValueNode* EnsureTagged(MaglevCompilationUnit& compilation_unit,
                           ValueNode* value) {
     switch (value->properties().value_representation()) {
       case ValueRepresentation::kTagged:
         return value;
       case ValueRepresentation::kInt32:
-        return TagValue(compilation_unit, value);
+        return FromInt32ToTagged(compilation_unit, value);
       case ValueRepresentation::kFloat64:
-        // TOOD(victorgomes): Support Float64.
-        UNREACHABLE();
+        return FromFloat64ToTagged(compilation_unit, value);
     }
   }
 

src/maglev/maglev-ir.cc

@@ -387,6 +387,19 @@ void SmiConstant::PrintParams(std::ostream& os,
   os << "(" << value() << ")";
 }
 
+void Float64Constant::AllocateVreg(MaglevVregAllocationState* vreg_state,
+                                   const ProcessingState& state) {
+  DefineAsRegister(vreg_state, this);
+}
+void Float64Constant::GenerateCode(MaglevCodeGenState* code_gen_state,
+                                   const ProcessingState& state) {
+  __ Move(ToDoubleRegister(result()), value());
+}
+void Float64Constant::PrintParams(std::ostream& os,
+                                  MaglevGraphLabeller* graph_labeller) const {
+  os << "(" << value() << ")";
+}
+
 void Constant::AllocateVreg(MaglevVregAllocationState* vreg_state,
                             const ProcessingState& state) {
   DefineAsRegister(vreg_state, this);
@@ -854,6 +867,16 @@ void CheckedFloat64Unbox::GenerateCode(MaglevCodeGenState* code_gen_state,
   __ bind(&done);
 }
 
+void ChangeInt32ToFloat64::AllocateVreg(MaglevVregAllocationState* vreg_state,
+                                        const ProcessingState& state) {
+  UseRegister(input());
+  DefineAsRegister(vreg_state, this);
+}
+void ChangeInt32ToFloat64::GenerateCode(MaglevCodeGenState* code_gen_state,
+                                        const ProcessingState& state) {
+  __ Cvtlsi2sd(ToDoubleRegister(result()), ToRegister(input()));
+}
+
 void Float64Add::AllocateVreg(MaglevVregAllocationState* vreg_state,
                               const ProcessingState& state) {
   UseRegister(left_input());

src/maglev/maglev-ir.h

@@ -82,6 +82,8 @@ class CompactInterpreterFrameState;
   V(CheckedSmiUntag)       \
   V(Int32AddWithOverflow)  \
   V(Int32Constant)         \
+  V(Float64Constant)       \
+  V(ChangeInt32ToFloat64)  \
   V(Float64Box)            \
   V(CheckedFloat64Unbox)   \
   V(Float64Add)            \
@@ -734,7 +736,7 @@ class ValueNode : public Node {
 
   bool is_spilled() const {
     DCHECK_EQ(state_, kSpillOrHint);
-    return spill_or_hint_.IsStackSlot();
+    return spill_or_hint_.IsAnyStackSlot();
   }
 
   void Spill(compiler::AllocatedOperand operand) {
@@ -747,6 +749,7 @@ class ValueNode : public Node {
 #endif  // DEBUG
     DCHECK(operand.IsAnyStackSlot());
     spill_or_hint_ = operand;
+    DCHECK(spill_or_hint_.IsAnyStackSlot());
   }
 
   compiler::AllocatedOperand spill_slot() const {
@@ -833,12 +836,6 @@ 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());
@@ -1097,6 +1094,25 @@ class Int32Constant : public FixedInputValueNodeT<0, Int32Constant> {
   const int32_t value_;
 };
 
+class Float64Constant : public FixedInputValueNodeT<0, Float64Constant> {
+  using Base = FixedInputValueNodeT<0, Float64Constant>;
+
+ public:
+  explicit Float64Constant(uint32_t bitfield, double value)
+      : Base(bitfield), value_(value) {}
+
+  static constexpr OpProperties kProperties = OpProperties::Float64();
+
+  double value() const { return value_; }
+
+  void AllocateVreg(MaglevVregAllocationState*, const ProcessingState&);
+  void GenerateCode(MaglevCodeGenState*, const ProcessingState&);
+  void PrintParams(std::ostream&, MaglevGraphLabeller*) const;
+
+ private:
+  const double value_;
+};
+
 class Int32AddWithOverflow
     : public FixedInputValueNodeT<2, Int32AddWithOverflow> {
   using Base = FixedInputValueNodeT<2, Int32AddWithOverflow>;
@@ -1132,6 +1148,22 @@ class Float64Box : public FixedInputValueNodeT<1, Float64Box> {
   void PrintParams(std::ostream&, MaglevGraphLabeller*) const {}
 };
 
+class ChangeInt32ToFloat64
+    : public FixedInputValueNodeT<1, ChangeInt32ToFloat64> {
+  using Base = FixedInputValueNodeT<1, ChangeInt32ToFloat64>;
+
+ public:
+  explicit ChangeInt32ToFloat64(uint32_t bitfield) : Base(bitfield) {}
+
+  static constexpr OpProperties kProperties = 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 CheckedFloat64Unbox
     : public FixedInputValueNodeT<1, CheckedFloat64Unbox> {
   using Base = FixedInputValueNodeT<1, CheckedFloat64Unbox>;

src/maglev/maglev-regalloc.cc

@@ -736,9 +736,7 @@ void StraightForwardRegisterAllocator::AllocateSpillSlot(ValueNode* node) {
   // TODO(v8:7700): We will need a new class of SpillSlots for doubles in 32-bit
   // architectures.
   SpillSlots& slots = is_tagged ? tagged_ : untagged_;
-  MachineRepresentation representation = is_tagged
-                                             ? MachineRepresentation::kTagged
-                                             : MachineRepresentation::kWord64;
+  MachineRepresentation representation = node->GetMachineRepresentation();
   if (slots.free_slots.empty()) {
     free_slot = slots.top++;
   } else {

test/mjsunit/maglev/add-number.js

@@ -60,3 +60,29 @@
   assertEquals("4.2!", add(4.2, "!"));
   assertFalse(isMaglevved(add));
 })();
+
+// Emit FloatAdd through SmiAdd bytecode.
+(function() {
+  function inc(x) {
+    return x + 1
+  }
+
+  %PrepareFunctionForOptimization(inc);
+  assertEquals(4.2, inc(3.2));
+
+  %OptimizeMaglevOnNextCall(inc);
+  assertEquals(4.2, inc(3.2));
+})();
+
+// Force the input of FloatAdd to be int32.
+(function() {
+  function add(x, y, z) {
+    return (x + y) + z;
+  }
+
+  %PrepareFunctionForOptimization(add);
+  assertEquals(4.2, add(1, 3, 0.2));
+
+  %OptimizeMaglevOnNextCall(add);
+  assertEquals(4.2, add(1, 3, 0.2));
+})();

test/mjsunit/maglev/spill-double.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
+
+// This tests that we can lazy deopt with a double spilled.
+
+const value = 2.2;
+
+function test(x) {
+  return x;
+}
+
+function g(x) {
+  assertEquals(value + 1, x);
+}
+
+function f(b, a) {
+  var x = a + 1;
+  if (test(b)) { // This forces x to be spilled.
+    g(x); // When we lazy deopt, we call g with the materialized
+          // value from the stack.
+  }
+  return x + 1;
+}
+
+%PrepareFunctionForOptimization(f);
+assertEquals(4.2, f(false, value));
+
+%OptimizeMaglevOnNextCall(f);
+assertEquals(4.2, f(false, value));
+assertTrue(isMaglevved(f));
+
+// We should deopt here.
+assertEquals(4.2, f(true, value));
+assertFalse(isMaglevved(f));