[turbofan] ChangeFloat64ToTagged shouldn't canonicalize.

This matches current Crankshaft/fullcodegen behavior more closely and
thus reduces the chances that we run into unnecessary polymorphism due
to the field representation tracking in our object model.

R=jarin@chromium.org
BUG=v8:5267

Committed: https://chromium.googlesource.com/v8/v8/+/6a939714e991ebf10d56ddbd2869325cca99c0ef
Committed: https://crrev.com/ee158e6c4cc896479a32245432a3c2fdd31bcb73
Review-Url: https://codereview.chromium.org/2367593003
Cr-Original-Commit-Position: refs/heads/master@{#39692}
Cr-Commit-Position: refs/heads/master@{#39748}

src/compiler/effect-control-linearizer.cc

@@ -776,75 +776,8 @@ 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);
-  Node* check_same = graph()->NewNode(
-      machine()->Float64Equal(), value,
-      graph()->NewNode(machine()->ChangeInt32ToFloat64(), value32));
-  Node* branch_same = graph()->NewNode(common()->Branch(), check_same, control);
-
-  Node* if_smi = graph()->NewNode(common()->IfTrue(), branch_same);
-  Node* vsmi;
-  Node* if_box = graph()->NewNode(common()->IfFalse(), branch_same);
-
-  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);
-
-    // 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);
-
-    // 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.
-  if (machine()->Is64()) {
-    vsmi = ChangeInt32ToSmi(value32);
-  } else {
-    Node* smi_tag = graph()->NewNode(machine()->Int32AddWithOverflow(), value32,
-                                     value32, if_smi);
-
-    Node* check_ovf =
-        graph()->NewNode(common()->Projection(1), smi_tag, if_smi);
-    Node* branch_ovf = graph()->NewNode(common()->Branch(BranchHint::kFalse),
-                                        check_ovf, if_smi);
-
-    Node* if_ovf = graph()->NewNode(common()->IfTrue(), branch_ovf);
-    if_box = graph()->NewNode(common()->Merge(2), if_ovf, if_box);
-
-    if_smi = graph()->NewNode(common()->IfFalse(), branch_ovf);
-    vsmi = graph()->NewNode(common()->Projection(0), smi_tag, if_smi);
-  }
-
-  // Allocate the box for the {value}.
-  ValueEffectControl box = AllocateHeapNumberWithValue(value, effect, if_box);
-
-  control = graph()->NewNode(common()->Merge(2), if_smi, box.control);
-  value = graph()->NewNode(common()->Phi(MachineRepresentation::kTagged, 2),
-                           vsmi, box.value, control);
-  effect =
-      graph()->NewNode(common()->EffectPhi(2), effect, box.effect, control);
-  return ValueEffectControl(value, effect, control);
+  return AllocateHeapNumberWithValue(value, effect, control);
 }
 
 EffectControlLinearizer::ValueEffectControl

src/compiler/representation-change.cc

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

src/compiler/simplified-operator.cc

@@ -208,8 +208,7 @@ CheckFloat64HoleMode CheckFloat64HoleModeOf(const Operator* op) {
 }
 
 CheckForMinusZeroMode CheckMinusZeroModeOf(const Operator* op) {
-  DCHECK(op->opcode() == IrOpcode::kChangeFloat64ToTagged ||
-         op->opcode() == IrOpcode::kCheckedInt32Mul ||
+  DCHECK(op->opcode() == IrOpcode::kCheckedInt32Mul ||
          op->opcode() == IrOpcode::kCheckedFloat64ToInt32 ||
          op->opcode() == IrOpcode::kCheckedTaggedToInt32);
   return OpParameter<CheckForMinusZeroMode>(op);
@@ -399,6 +398,7 @@ PretenureFlag PretenureFlagOf(const Operator* op) {
   V(ChangeTaggedToInt32, Operator::kNoProperties, 1, 0)          \
   V(ChangeTaggedToUint32, Operator::kNoProperties, 1, 0)         \
   V(ChangeTaggedToFloat64, Operator::kNoProperties, 1, 0)        \
+  V(ChangeFloat64ToTagged, Operator::kNoProperties, 1, 0)        \
   V(ChangeInt31ToTaggedSigned, Operator::kNoProperties, 1, 0)    \
   V(ChangeInt32ToTagged, Operator::kNoProperties, 1, 0)          \
   V(ChangeUint32ToTagged, Operator::kNoProperties, 1, 0)         \
@@ -475,19 +475,6 @@ struct SimplifiedOperatorGlobalCache final {
   };
   ArrayBufferWasNeuteredOperator kArrayBufferWasNeutered;
 
-  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> {
@@ -634,18 +621,6 @@ CHECKED_OP_LIST(GET_FROM_CACHE)
 GET_FROM_CACHE(ArrayBufferWasNeutered)
 #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

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

src/js/regexp.js

@@ -178,11 +178,20 @@ function RegExpExecJS(string) {
   var sticky = TO_BOOLEAN(REGEXP_STICKY(this));
   var updateLastIndex = global || sticky;
   if (updateLastIndex) {
-    lastIndex = TO_LENGTH(this.lastIndex);
+    // TODO(jgruber): This is actually ToLength in the spec, but we bailout
+    // to the runtime in %_RegExpExec if lastIndex is not a Smi, so we are
+    // smart here and trick both TurboFan and Crankshaft to produce a Smi.
+    // This is a terrible hack, and correct for subtle reasons; it's a clear
+    // indicator that we need a predictable RegExp implementation where we
+    // don't need to add specific work-arounds for certain compiler issues.
+    lastIndex = +this.lastIndex;
     if (lastIndex > string.length) {
       this.lastIndex = 0;
       return null;
+    } else if (lastIndex <= 0) {
+      lastIndex = 0;
     }
+    lastIndex = lastIndex|0;
   } else {
     lastIndex = 0;
   }

src/runtime/runtime-test.cc

@@ -420,8 +420,8 @@ RUNTIME_FUNCTION(Runtime_SetAllocationTimeout) {
   SealHandleScope shs(isolate);
   DCHECK(args.length() == 2 || args.length() == 3);
 #ifdef DEBUG
-  CONVERT_SMI_ARG_CHECKED(interval, 0);
-  CONVERT_SMI_ARG_CHECKED(timeout, 1);
+  CONVERT_INT32_ARG_CHECKED(interval, 0);
+  CONVERT_INT32_ARG_CHECKED(timeout, 1);
   isolate->heap()->set_allocation_timeout(timeout);
   FLAG_gc_interval = interval;
   if (args.length() == 3) {

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

@@ -97,10 +97,6 @@ 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
 
 
@@ -191,13 +187,11 @@ TEST_F(SimplifiedOperatorReducerTest, ChangeTaggedToBitWithChangeBitToTagged) {
 // ChangeFloat64ToTagged
 
 TEST_F(SimplifiedOperatorReducerTest, ChangeFloat64ToTaggedWithConstant) {
-  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)));
-    }
+  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)));
   }
 }
 
@@ -222,13 +216,11 @@ TEST_F(SimplifiedOperatorReducerTest, ChangeInt32ToTaggedWithConstant) {
 TEST_F(SimplifiedOperatorReducerTest,
        ChangeTaggedToFloat64WithChangeFloat64ToTagged) {
   Node* param0 = Parameter(0);
-  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());
-  }
+  Reduction reduction = Reduce(graph()->NewNode(
+      simplified()->ChangeTaggedToFloat64(),
+      graph()->NewNode(simplified()->ChangeFloat64ToTagged(), param0)));
+  ASSERT_TRUE(reduction.Changed());
+  EXPECT_EQ(param0, reduction.replacement());
 }
 
 TEST_F(SimplifiedOperatorReducerTest,
@@ -279,13 +271,11 @@ TEST_F(SimplifiedOperatorReducerTest, ChangeTaggedToFloat64WithNaNConstant) {
 TEST_F(SimplifiedOperatorReducerTest,
        ChangeTaggedToInt32WithChangeFloat64ToTagged) {
   Node* param0 = Parameter(0);
-  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));
-  }
+  Reduction reduction = Reduce(graph()->NewNode(
+      simplified()->ChangeTaggedToInt32(),
+      graph()->NewNode(simplified()->ChangeFloat64ToTagged(), param0)));
+  ASSERT_TRUE(reduction.Changed());
+  EXPECT_THAT(reduction.replacement(), IsChangeFloat64ToInt32(param0));
 }
 
 TEST_F(SimplifiedOperatorReducerTest,
@@ -305,13 +295,11 @@ TEST_F(SimplifiedOperatorReducerTest,
 TEST_F(SimplifiedOperatorReducerTest,
        ChangeTaggedToUint32WithChangeFloat64ToTagged) {
   Node* param0 = Parameter(0);
-  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));
-  }
+  Reduction reduction = Reduce(graph()->NewNode(
+      simplified()->ChangeTaggedToUint32(),
+      graph()->NewNode(simplified()->ChangeFloat64ToTagged(), param0)));
+  ASSERT_TRUE(reduction.Changed());
+  EXPECT_THAT(reduction.replacement(), IsChangeFloat64ToUint32(param0));
 }
 
 TEST_F(SimplifiedOperatorReducerTest,
@@ -331,13 +319,11 @@ TEST_F(SimplifiedOperatorReducerTest,
 TEST_F(SimplifiedOperatorReducerTest,
        TruncateTaggedToWord3WithChangeFloat64ToTagged) {
   Node* param0 = Parameter(0);
-  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));
-  }
+  Reduction reduction = Reduce(graph()->NewNode(
+      simplified()->TruncateTaggedToWord32(),
+      graph()->NewNode(simplified()->ChangeFloat64ToTagged(), param0)));
+  ASSERT_TRUE(reduction.Changed());
+  EXPECT_THAT(reduction.replacement(), IsTruncateFloat64ToWord32(param0));
 }
 
 TEST_F(SimplifiedOperatorReducerTest, TruncateTaggedToWord32WithConstant) {