[turbofan] Let ChangeFloat64ToTagged canonicalize to Smi if possible.

When the incoming value to ChangeFloat64ToTagged is in Smi range, we
represent it as Smi instead of a HeapNumber. This addresses a range of
problems where TurboFan unnecessarily deoptimizes because an operation
learned Smi feedback in Ignition, but was then confronted with a tagged
HeapNumber in TurboFan, just because the value was also represented as
unboxed double somewhere in the meantime.

BUG=v8:6256
R=yangguo@chromium.org

Review-Url: https://codereview.chromium.org/2815283002
Cr-Commit-Position: refs/heads/master@{#44631}

src/compiler/effect-control-linearizer.cc

@@ -829,8 +829,62 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node,
 #define __ gasm()->
 
 Node* EffectControlLinearizer::LowerChangeFloat64ToTagged(Node* node) {
+  CheckForMinusZeroMode mode = CheckMinusZeroModeOf(node->op());
   Node* value = node->InputAt(0);
-  return AllocateHeapNumberWithValue(value);
+
+  auto done = __ MakeLabel<2>(MachineRepresentation::kTagged);
+  auto if_heapnumber =
+      __ MakeLabelFor(GraphAssemblerLabelType::kNonDeferred,
+                      1 + (mode == CheckForMinusZeroMode::kCheckForMinusZero) +
+                          !machine()->Is64());
+  auto if_int32 = __ MakeLabel<1>();
+
+  Node* value32 = __ RoundFloat64ToInt32(value);
+  __ GotoIf(__ Float64Equal(value, __ ChangeInt32ToFloat64(value32)),
+            &if_int32);
+  __ Goto(&if_heapnumber);
+
+  __ Bind(&if_int32);
+  {
+    if (mode == CheckForMinusZeroMode::kCheckForMinusZero) {
+      Node* zero = __ Int32Constant(0);
+      auto if_zero = __ MakeDeferredLabel<1>();
+      auto if_smi = __ MakeLabel<2>();
+
+      __ GotoIf(__ Word32Equal(value32, zero), &if_zero);
+      __ Goto(&if_smi);
+
+      __ Bind(&if_zero);
+      {
+        // In case of 0, we need to check the high bits for the IEEE -0 pattern.
+        __ GotoIf(__ Int32LessThan(__ Float64ExtractHighWord32(value), zero),
+                  &if_heapnumber);
+        __ Goto(&if_smi);
+      }
+
+      __ Bind(&if_smi);
+    }
+
+    if (machine()->Is64()) {
+      Node* value_smi = ChangeInt32ToSmi(value32);
+      __ Goto(&done, value_smi);
+    } else {
+      Node* add = __ Int32AddWithOverflow(value32, value32);
+      Node* ovf = __ Projection(1, add);
+      __ GotoIf(ovf, &if_heapnumber);
+      Node* value_smi = __ Projection(0, add);
+      __ Goto(&done, value_smi);
+    }
+  }
+
+  __ Bind(&if_heapnumber);
+  {
+    Node* value_number = AllocateHeapNumberWithValue(value);
+    __ Goto(&done, value_number);
+  }
+
+  __ Bind(&done);
+  return done.PhiAt(0);
 }
 
 Node* EffectControlLinearizer::LowerChangeFloat64ToTaggedPointer(Node* node) {

src/compiler/representation-change.cc

@@ -436,7 +436,10 @@ Node* RepresentationChanger::GetTaggedRepresentationFor(
   } else if (output_rep ==
              MachineRepresentation::kFloat32) {  // float32 -> float64 -> tagged
     node = InsertChangeFloat32ToFloat64(node);
-    op = simplified()->ChangeFloat64ToTagged();
+    op = simplified()->ChangeFloat64ToTagged(
+        output_type->Maybe(Type::MinusZero())
+            ? CheckForMinusZeroMode::kCheckForMinusZero
+            : CheckForMinusZeroMode::kDontCheckForMinusZero);
   } else if (output_rep == MachineRepresentation::kFloat64) {
     if (output_type->Is(Type::Signed31())) {  // float64 -> int32 -> tagged
       node = InsertChangeFloat64ToInt32(node);
@@ -450,7 +453,10 @@ Node* RepresentationChanger::GetTaggedRepresentationFor(
       node = InsertChangeFloat64ToUint32(node);
       op = simplified()->ChangeUint32ToTagged();
     } else {
-      op = simplified()->ChangeFloat64ToTagged();
+      op = simplified()->ChangeFloat64ToTagged(
+          output_type->Maybe(Type::MinusZero())
+              ? CheckForMinusZeroMode::kCheckForMinusZero
+              : CheckForMinusZeroMode::kDontCheckForMinusZero);
     }
   } else {
     return TypeError(node, output_rep, output_type,

src/compiler/simplified-operator.cc

@@ -213,7 +213,8 @@ CheckFloat64HoleMode CheckFloat64HoleModeOf(const Operator* op) {
 }
 
 CheckForMinusZeroMode CheckMinusZeroModeOf(const Operator* op) {
-  DCHECK(op->opcode() == IrOpcode::kCheckedInt32Mul ||
+  DCHECK(op->opcode() == IrOpcode::kChangeFloat64ToTagged ||
+         op->opcode() == IrOpcode::kCheckedInt32Mul ||
          op->opcode() == IrOpcode::kCheckedFloat64ToInt32 ||
          op->opcode() == IrOpcode::kCheckedTaggedToInt32);
   return OpParameter<CheckForMinusZeroMode>(op);
@@ -488,7 +489,6 @@ UnicodeEncoding UnicodeEncodingOf(const Operator* op) {
   V(ChangeTaggedToUint32, Operator::kNoProperties, 1, 0)         \
   V(ChangeTaggedToFloat64, Operator::kNoProperties, 1, 0)        \
   V(ChangeTaggedToTaggedSigned, Operator::kNoProperties, 1, 0)   \
-  V(ChangeFloat64ToTagged, Operator::kNoProperties, 1, 0)        \
   V(ChangeFloat64ToTaggedPointer, Operator::kNoProperties, 1, 0) \
   V(ChangeInt31ToTaggedSigned, Operator::kNoProperties, 1, 0)    \
   V(ChangeInt32ToTagged, Operator::kNoProperties, 1, 0)          \
@@ -600,6 +600,19 @@ struct SimplifiedOperatorGlobalCache final {
   };
   NewUnmappedArgumentsElementsOperator kNewUnmappedArgumentsElements;
 
+  template <CheckForMinusZeroMode kMode>
+  struct ChangeFloat64ToTaggedOperator final
+      : public Operator1<CheckForMinusZeroMode> {
+    ChangeFloat64ToTaggedOperator()
+        : Operator1<CheckForMinusZeroMode>(
+              IrOpcode::kChangeFloat64ToTagged, Operator::kPure,
+              "ChangeFloat64ToTagged", 1, 0, 0, 1, 0, 0, kMode) {}
+  };
+  ChangeFloat64ToTaggedOperator<CheckForMinusZeroMode::kCheckForMinusZero>
+      kChangeFloat64ToTaggedCheckForMinusZeroOperator;
+  ChangeFloat64ToTaggedOperator<CheckForMinusZeroMode::kDontCheckForMinusZero>
+      kChangeFloat64ToTaggedDontCheckForMinusZeroOperator;
+
   template <CheckForMinusZeroMode kMode>
   struct CheckedInt32MulOperator final
       : public Operator1<CheckForMinusZeroMode> {
@@ -757,6 +770,18 @@ GET_FROM_CACHE(ArgumentsFrame)
 GET_FROM_CACHE(NewUnmappedArgumentsElements)
 #undef GET_FROM_CACHE
 
+const Operator* SimplifiedOperatorBuilder::ChangeFloat64ToTagged(
+    CheckForMinusZeroMode mode) {
+  switch (mode) {
+    case CheckForMinusZeroMode::kCheckForMinusZero:
+      return &cache_.kChangeFloat64ToTaggedCheckForMinusZeroOperator;
+    case CheckForMinusZeroMode::kDontCheckForMinusZero:
+      return &cache_.kChangeFloat64ToTaggedDontCheckForMinusZeroOperator;
+  }
+  UNREACHABLE();
+  return nullptr;
+}
+
 const Operator* SimplifiedOperatorBuilder::CheckedInt32Mul(
     CheckForMinusZeroMode mode) {
   switch (mode) {

src/compiler/simplified-operator.h

@@ -395,7 +395,7 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
   const Operator* ChangeInt31ToTaggedSigned();
   const Operator* ChangeInt32ToTagged();
   const Operator* ChangeUint32ToTagged();
-  const Operator* ChangeFloat64ToTagged();
+  const Operator* ChangeFloat64ToTagged(CheckForMinusZeroMode);
   const Operator* ChangeFloat64ToTaggedPointer();
   const Operator* ChangeTaggedToBit();
   const Operator* ChangeBitToTagged();

test/unittests/compiler/simplified-operator-reducer-unittest.cc

@@ -97,6 +97,10 @@ const double kNaNs[] = {-std::numeric_limits<double>::quiet_NaN(),
                         bit_cast<double>(V8_UINT64_C(0x7FFFFFFFFFFFFFFF)),
                         bit_cast<double>(V8_UINT64_C(0xFFFFFFFFFFFFFFFF))};
 
+const CheckForMinusZeroMode kCheckForMinusZeroModes[] = {
+    CheckForMinusZeroMode::kDontCheckForMinusZero,
+    CheckForMinusZeroMode::kCheckForMinusZero};
+
 }  // namespace
 
 
@@ -187,11 +191,13 @@ TEST_F(SimplifiedOperatorReducerTest, ChangeTaggedToBitWithChangeBitToTagged) {
 // ChangeFloat64ToTagged
 
 TEST_F(SimplifiedOperatorReducerTest, ChangeFloat64ToTaggedWithConstant) {
-  TRACED_FOREACH(double, n, kFloat64Values) {
-    Reduction reduction = Reduce(graph()->NewNode(
-        simplified()->ChangeFloat64ToTagged(), Float64Constant(n)));
-    ASSERT_TRUE(reduction.Changed());
-    EXPECT_THAT(reduction.replacement(), IsNumberConstant(BitEq(n)));
+  TRACED_FOREACH(CheckForMinusZeroMode, mode, kCheckForMinusZeroModes) {
+    TRACED_FOREACH(double, n, kFloat64Values) {
+      Reduction reduction = Reduce(graph()->NewNode(
+          simplified()->ChangeFloat64ToTagged(mode), Float64Constant(n)));
+      ASSERT_TRUE(reduction.Changed());
+      EXPECT_THAT(reduction.replacement(), IsNumberConstant(BitEq(n)));
+    }
   }
 }
 
@@ -216,11 +222,13 @@ TEST_F(SimplifiedOperatorReducerTest, ChangeInt32ToTaggedWithConstant) {
 TEST_F(SimplifiedOperatorReducerTest,
        ChangeTaggedToFloat64WithChangeFloat64ToTagged) {
   Node* param0 = Parameter(0);
-  Reduction reduction = Reduce(graph()->NewNode(
-      simplified()->ChangeTaggedToFloat64(),
-      graph()->NewNode(simplified()->ChangeFloat64ToTagged(), param0)));
-  ASSERT_TRUE(reduction.Changed());
-  EXPECT_EQ(param0, reduction.replacement());
+  TRACED_FOREACH(CheckForMinusZeroMode, mode, kCheckForMinusZeroModes) {
+    Reduction reduction = Reduce(graph()->NewNode(
+        simplified()->ChangeTaggedToFloat64(),
+        graph()->NewNode(simplified()->ChangeFloat64ToTagged(mode), param0)));
+    ASSERT_TRUE(reduction.Changed());
+    EXPECT_EQ(param0, reduction.replacement());
+  }
 }
 
 TEST_F(SimplifiedOperatorReducerTest,
@@ -271,11 +279,13 @@ TEST_F(SimplifiedOperatorReducerTest, ChangeTaggedToFloat64WithNaNConstant) {
 TEST_F(SimplifiedOperatorReducerTest,
        ChangeTaggedToInt32WithChangeFloat64ToTagged) {
   Node* param0 = Parameter(0);
-  Reduction reduction = Reduce(graph()->NewNode(
-      simplified()->ChangeTaggedToInt32(),
-      graph()->NewNode(simplified()->ChangeFloat64ToTagged(), param0)));
-  ASSERT_TRUE(reduction.Changed());
-  EXPECT_THAT(reduction.replacement(), IsChangeFloat64ToInt32(param0));
+  TRACED_FOREACH(CheckForMinusZeroMode, mode, kCheckForMinusZeroModes) {
+    Reduction reduction = Reduce(graph()->NewNode(
+        simplified()->ChangeTaggedToInt32(),
+        graph()->NewNode(simplified()->ChangeFloat64ToTagged(mode), param0)));
+    ASSERT_TRUE(reduction.Changed());
+    EXPECT_THAT(reduction.replacement(), IsChangeFloat64ToInt32(param0));
+  }
 }
 
 TEST_F(SimplifiedOperatorReducerTest,
@@ -295,11 +305,13 @@ TEST_F(SimplifiedOperatorReducerTest,
 TEST_F(SimplifiedOperatorReducerTest,
        ChangeTaggedToUint32WithChangeFloat64ToTagged) {
   Node* param0 = Parameter(0);
-  Reduction reduction = Reduce(graph()->NewNode(
-      simplified()->ChangeTaggedToUint32(),
-      graph()->NewNode(simplified()->ChangeFloat64ToTagged(), param0)));
-  ASSERT_TRUE(reduction.Changed());
-  EXPECT_THAT(reduction.replacement(), IsChangeFloat64ToUint32(param0));
+  TRACED_FOREACH(CheckForMinusZeroMode, mode, kCheckForMinusZeroModes) {
+    Reduction reduction = Reduce(graph()->NewNode(
+        simplified()->ChangeTaggedToUint32(),
+        graph()->NewNode(simplified()->ChangeFloat64ToTagged(mode), param0)));
+    ASSERT_TRUE(reduction.Changed());
+    EXPECT_THAT(reduction.replacement(), IsChangeFloat64ToUint32(param0));
+  }
 }
 
 TEST_F(SimplifiedOperatorReducerTest,
@@ -319,11 +331,13 @@ TEST_F(SimplifiedOperatorReducerTest,
 TEST_F(SimplifiedOperatorReducerTest,
        TruncateTaggedToWord3WithChangeFloat64ToTagged) {
   Node* param0 = Parameter(0);
-  Reduction reduction = Reduce(graph()->NewNode(
-      simplified()->TruncateTaggedToWord32(),
-      graph()->NewNode(simplified()->ChangeFloat64ToTagged(), param0)));
-  ASSERT_TRUE(reduction.Changed());
-  EXPECT_THAT(reduction.replacement(), IsTruncateFloat64ToWord32(param0));
+  TRACED_FOREACH(CheckForMinusZeroMode, mode, kCheckForMinusZeroModes) {
+    Reduction reduction = Reduce(graph()->NewNode(
+        simplified()->TruncateTaggedToWord32(),
+        graph()->NewNode(simplified()->ChangeFloat64ToTagged(mode), param0)));
+    ASSERT_TRUE(reduction.Changed());
+    EXPECT_THAT(reduction.replacement(), IsTruncateFloat64ToWord32(param0));
+  }
 }
 
 TEST_F(SimplifiedOperatorReducerTest, TruncateTaggedToWord32WithConstant) {