[turbofan] Avoid unnecessary minus zero checks for Float64->Tagged.

When we change representation from Float64 to Tagged and we know that
the input value can never be -0, we don't need to bother introducing
the check for -0 during effect/control linearization.

R=jarin@chromium.org

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

src/compiler/effect-control-linearizer.cc

@@ -762,6 +762,7 @@ bool EffectControlLinearizer::TryWireInStateEffect(Node* node,
 EffectControlLinearizer::ValueEffectControl
 EffectControlLinearizer::LowerChangeFloat64ToTagged(Node* node, Node* effect,
                                                     Node* control) {
+  CheckForMinusZeroMode mode = CheckMinusZeroModeOf(node->op());
   Node* value = node->InputAt(0);
 
   Node* value32 = graph()->NewNode(machine()->RoundFloat64ToInt32(), value);
@@ -774,29 +775,32 @@ EffectControlLinearizer::LowerChangeFloat64ToTagged(Node* node, Node* effect,
   Node* vsmi;
   Node* if_box = graph()->NewNode(common()->IfFalse(), branch_same);
 
-  // Check if {value} is -0.
-  Node* check_zero = graph()->NewNode(machine()->Word32Equal(), value32,
-                                      jsgraph()->Int32Constant(0));
-  Node* branch_zero = graph()->NewNode(common()->Branch(BranchHint::kFalse),
-                                       check_zero, if_smi);
+  if (mode == CheckForMinusZeroMode::kCheckForMinusZero) {
+    // Check if {value} is -0.
+    Node* check_zero = graph()->NewNode(machine()->Word32Equal(), value32,
+                                        jsgraph()->Int32Constant(0));
+    Node* branch_zero = graph()->NewNode(common()->Branch(BranchHint::kFalse),
+                                         check_zero, if_smi);
 
-  Node* if_zero = graph()->NewNode(common()->IfTrue(), branch_zero);
-  Node* if_notzero = graph()->NewNode(common()->IfFalse(), branch_zero);
+    Node* if_zero = graph()->NewNode(common()->IfTrue(), branch_zero);
+    Node* if_notzero = graph()->NewNode(common()->IfFalse(), branch_zero);
 
-  // In case of 0, we need to check the high bits for the IEEE -0 pattern.
-  Node* check_negative = graph()->NewNode(
-      machine()->Int32LessThan(),
-      graph()->NewNode(machine()->Float64ExtractHighWord32(), value),
-      jsgraph()->Int32Constant(0));
-  Node* branch_negative = graph()->NewNode(common()->Branch(BranchHint::kFalse),
-                                           check_negative, if_zero);
+    // In case of 0, we need to check the high bits for the IEEE -0 pattern.
+    Node* check_negative = graph()->NewNode(
+        machine()->Int32LessThan(),
+        graph()->NewNode(machine()->Float64ExtractHighWord32(), value),
+        jsgraph()->Int32Constant(0));
+    Node* branch_negative = graph()->NewNode(
+        common()->Branch(BranchHint::kFalse), check_negative, if_zero);
 
-  Node* if_negative = graph()->NewNode(common()->IfTrue(), branch_negative);
-  Node* if_notnegative = graph()->NewNode(common()->IfFalse(), branch_negative);
+    Node* if_negative = graph()->NewNode(common()->IfTrue(), branch_negative);
+    Node* if_notnegative =
+        graph()->NewNode(common()->IfFalse(), branch_negative);
 
-  // We need to create a box for negative 0.
-  if_smi = graph()->NewNode(common()->Merge(2), if_notzero, if_notnegative);
-  if_box = graph()->NewNode(common()->Merge(2), if_box, if_negative);
+    // We need to create a box for negative 0.
+    if_smi = graph()->NewNode(common()->Merge(2), if_notzero, if_notnegative);
+    if_box = graph()->NewNode(common()->Merge(2), if_box, if_negative);
+  }
 
   // On 64-bit machines we can just wrap the 32-bit integer in a smi, for 32-bit
   // machines we need to deal with potential overflow and fallback to boxing.

src/compiler/representation-change.cc

@@ -227,8 +227,10 @@ Node* RepresentationChanger::GetTaggedRepresentationFor(
   } else if (output_rep ==
              MachineRepresentation::kFloat32) {  // float32 -> float64 -> tagged
     node = InsertChangeFloat32ToFloat64(node);
-    // TODO(bmeurer): Pass -0 hint to ChangeFloat64ToTagged.
-    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);
@@ -242,8 +244,10 @@ Node* RepresentationChanger::GetTaggedRepresentationFor(
       node = InsertChangeFloat64ToUint32(node);
       op = simplified()->ChangeUint32ToTagged();
     } else {
-      // TODO(bmeurer): Pass -0 hint to ChangeFloat64ToTagged.
-      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

@@ -208,7 +208,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);
@@ -395,7 +396,6 @@ NumberOperationHint NumberOperationHintOf(const Operator* op) {
   V(ChangeInt31ToTaggedSigned, Operator::kNoProperties, 1, 0)    \
   V(ChangeInt32ToTagged, Operator::kNoProperties, 1, 0)          \
   V(ChangeUint32ToTagged, Operator::kNoProperties, 1, 0)         \
-  V(ChangeFloat64ToTagged, Operator::kNoProperties, 1, 0)        \
   V(ChangeTaggedToBit, Operator::kNoProperties, 1, 0)            \
   V(ChangeBitToTagged, Operator::kNoProperties, 1, 0)            \
   V(TruncateTaggedToWord32, Operator::kNoProperties, 1, 0)       \
@@ -458,6 +458,19 @@ struct SimplifiedOperatorGlobalCache final {
   CHECKED_OP_LIST(CHECKED)
 #undef CHECKED
 
+  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> {
@@ -603,6 +616,18 @@ PURE_OP_LIST(GET_FROM_CACHE)
 CHECKED_OP_LIST(GET_FROM_CACHE)
 #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

@@ -299,7 +299,7 @@ class SimplifiedOperatorBuilder final : public ZoneObject {
   const Operator* ChangeInt31ToTaggedSigned();
   const Operator* ChangeInt32ToTagged();
   const Operator* ChangeUint32ToTagged();
-  const Operator* ChangeFloat64ToTagged();
+  const Operator* ChangeFloat64ToTagged(CheckForMinusZeroMode);
   const Operator* ChangeTaggedToBit();
   const Operator* ChangeBitToTagged();
   const Operator* TruncateTaggedToWord32();

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
 
 
@@ -183,21 +187,20 @@ TEST_F(SimplifiedOperatorReducerTest, ChangeTaggedToBitWithChangeBitToTagged) {
   EXPECT_EQ(param0, reduction.replacement());
 }
 
-
 // -----------------------------------------------------------------------------
 // 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)));
+    }
   }
 }
 
-
 // -----------------------------------------------------------------------------
 // ChangeInt32ToTagged
 
@@ -219,14 +222,15 @@ 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,
        ChangeTaggedToFloat64WithChangeInt32ToTagged) {
   Node* param0 = Parameter(0);
@@ -272,18 +276,18 @@ TEST_F(SimplifiedOperatorReducerTest, ChangeTaggedToFloat64WithNaNConstant) {
 // -----------------------------------------------------------------------------
 // ChangeTaggedToInt32
 
-
 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,
        ChangeTaggedToInt32WithChangeInt32ToTagged) {
   Node* param0 = Parameter(0);
@@ -298,18 +302,18 @@ TEST_F(SimplifiedOperatorReducerTest,
 // -----------------------------------------------------------------------------
 // ChangeTaggedToUint32
 
-
 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,
        ChangeTaggedToUint32WithChangeUint32ToTagged) {
   Node* param0 = Parameter(0);
@@ -327,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) {

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

@@ -60,7 +60,6 @@ const PureOperator kPureOperators[] = {
     PURE(ChangeTaggedToFloat64, Operator::kNoProperties, 1),
     PURE(ChangeInt32ToTagged, Operator::kNoProperties, 1),
     PURE(ChangeUint32ToTagged, Operator::kNoProperties, 1),
-    PURE(ChangeFloat64ToTagged, Operator::kNoProperties, 1),
     PURE(ChangeTaggedToBit, Operator::kNoProperties, 1),
     PURE(ChangeBitToTagged, Operator::kNoProperties, 1),
     PURE(TruncateTaggedToWord32, Operator::kNoProperties, 1),