[turbofan] Make string comparisons effectful.

BUG=v8:4446
LOG=n

Review URL: https://codereview.chromium.org/1369313004

Cr-Commit-Position: refs/heads/master@{#31006}

diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc
index 6c080d9..c77d994 100644 (file)
--- a/src/compiler/js-typed-lowering.cc
+++ b/src/compiler/js-typed-lowering.cc
@@ -181,6 +181,32 @@ class JSBinopReduction final {
     return lowering_->Changed(node_);
   }
 
+  Reduction ChangeToStringComparisonOperator(const Operator* op,
+                                             bool invert = false) {
+    if (node_->op()->ControlInputCount() > 0) {
+      lowering_->RelaxControls(node_);
+    }
+    // String comparison operators need effect and control inputs, so copy them
+    // over.
+    Node* effect = NodeProperties::GetEffectInput(node_);
+    Node* control = NodeProperties::GetControlInput(node_);
+    node_->ReplaceInput(2, effect);
+    node_->ReplaceInput(3, control);
+
+    node_->TrimInputCount(4);
+    NodeProperties::ChangeOp(node_, op);
+
+    if (invert) {
+      // Insert a boolean-not to invert the value.
+      Node* value = graph()->NewNode(simplified()->BooleanNot(), node_);
+      node_->ReplaceUses(value);
+      // Note: ReplaceUses() smashes all uses, so smash it back here.
+      value->ReplaceInput(0, node_);
+      return lowering_->Replace(value);
+    }
+    return lowering_->Changed(node_);
+  }
+
   Reduction ChangeToPureOperator(const Operator* op, Type* type) {
     return ChangeToPureOperator(op, false, type);
   }
@@ -503,7 +529,8 @@ Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
       default:
         return NoChange();
     }
-    return r.ChangeToPureOperator(stringOp);
+    r.ChangeToStringComparisonOperator(stringOp);
+    return Changed(node);
   }
   if (r.OneInputCannotBe(Type::StringOrReceiver())) {
     const Operator* less_than;
@@ -557,7 +584,8 @@ Reduction JSTypedLowering::ReduceJSEqual(Node* node, bool invert) {
     return r.ChangeToPureOperator(simplified()->NumberEqual(), invert);
   }
   if (r.BothInputsAre(Type::String())) {
-    return r.ChangeToPureOperator(simplified()->StringEqual(), invert);
+    return r.ChangeToStringComparisonOperator(simplified()->StringEqual(),
+                                              invert);
   }
   if (r.BothInputsAre(Type::Receiver())) {
     return r.ChangeToPureOperator(
@@ -614,7 +642,8 @@ Reduction JSTypedLowering::ReduceJSStrictEqual(Node* node, bool invert) {
                                   invert);
   }
   if (r.BothInputsAre(Type::String())) {
-    return r.ChangeToPureOperator(simplified()->StringEqual(), invert);
+    return r.ChangeToStringComparisonOperator(simplified()->StringEqual(),
+                                              invert);
   }
   if (r.BothInputsAre(Type::Number())) {
     return r.ChangeToPureOperator(simplified()->NumberEqual(), invert);

diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc
index ecfba7f..7d495bf 100644 (file)
--- a/src/compiler/simplified-lowering.cc
+++ b/src/compiler/simplified-lowering.cc
@@ -1363,7 +1363,9 @@ Node* SimplifiedLowering::StringComparison(Node* node) {
   return graph()->NewNode(
       common()->Call(desc), jsgraph()->HeapConstant(callable.code()),
       NodeProperties::GetValueInput(node, 0),
-      NodeProperties::GetValueInput(node, 1), jsgraph()->NoContextConstant());
+      NodeProperties::GetValueInput(node, 1), jsgraph()->NoContextConstant(),
+      NodeProperties::GetEffectInput(node),
+      NodeProperties::GetControlInput(node));
 }
 
 
@@ -1627,23 +1629,49 @@ void SimplifiedLowering::DoShift(Node* node, Operator const* op) {
 }
 
 
+namespace {
+
+void ReplaceEffectUses(Node* node, Node* replacement) {
+  // Requires distinguishing between value and effect edges.
+  DCHECK(replacement->op()->EffectOutputCount() > 0);
+  for (Edge edge : node->use_edges()) {
+    if (NodeProperties::IsEffectEdge(edge)) {
+      edge.UpdateTo(replacement);
+    } else {
+      DCHECK(NodeProperties::IsValueEdge(edge));
+    }
+  }
+}
+
+}  // namespace
+
+
 void SimplifiedLowering::DoStringEqual(Node* node) {
-  node->ReplaceInput(0, StringComparison(node));
+  Node* comparison = StringComparison(node);
+  ReplaceEffectUses(node, comparison);
+  node->ReplaceInput(0, comparison);
   node->ReplaceInput(1, jsgraph()->SmiConstant(EQUAL));
+  node->TrimInputCount(2);
   NodeProperties::ChangeOp(node, machine()->WordEqual());
 }
 
 
 void SimplifiedLowering::DoStringLessThan(Node* node) {
-  node->ReplaceInput(0, StringComparison(node));
+  Node* comparison = StringComparison(node);
+  ReplaceEffectUses(node, comparison);
+  node->ReplaceInput(0, comparison);
   node->ReplaceInput(1, jsgraph()->SmiConstant(EQUAL));
+  node->TrimInputCount(2);
   NodeProperties::ChangeOp(node, machine()->IntLessThan());
 }
 
 
 void SimplifiedLowering::DoStringLessThanOrEqual(Node* node) {
-  node->ReplaceInput(0, StringComparison(node));
+  Node* comparison = StringComparison(node);
+  ReplaceEffectUses(node, comparison);
+  node->ReplaceInput(0, comparison);
   node->ReplaceInput(1, jsgraph()->SmiConstant(EQUAL));
+  node->TrimInputCount(2);
   NodeProperties::ChangeOp(node, machine()->IntLessThanOrEqual());
 }
 

diff --git a/src/compiler/simplified-operator.cc b/src/compiler/simplified-operator.cc
index dbb29ea..8432d21 100644 (file)
--- a/src/compiler/simplified-operator.cc
+++ b/src/compiler/simplified-operator.cc
@@ -174,9 +174,6 @@ const ElementAccess& ElementAccessOf(const Operator* op) {
   V(NumberToInt32, Operator::kNoProperties, 1)           \
   V(NumberToUint32, Operator::kNoProperties, 1)          \
   V(PlainPrimitiveToNumber, Operator::kNoProperties, 1)  \
-  V(StringEqual, Operator::kCommutative, 2)              \
-  V(StringLessThan, Operator::kNoProperties, 2)          \
-  V(StringLessThanOrEqual, Operator::kNoProperties, 2)   \
   V(ChangeTaggedToInt32, Operator::kNoProperties, 1)     \
   V(ChangeTaggedToUint32, Operator::kNoProperties, 1)    \
   V(ChangeTaggedToFloat64, Operator::kNoProperties, 1)   \
@@ -187,6 +184,10 @@ const ElementAccess& ElementAccessOf(const Operator* op) {
   V(ChangeBitToBool, Operator::kNoProperties, 1)         \
   V(ObjectIsSmi, Operator::kNoProperties, 1)
 
+#define NO_THROW_OP_LIST(V)                 \
+  V(StringEqual, Operator::kCommutative, 2) \
+  V(StringLessThan, Operator::kNoThrow, 2)  \
+  V(StringLessThanOrEqual, Operator::kNoThrow, 2)
 
 struct SimplifiedOperatorGlobalCache final {
 #define PURE(Name, properties, input_count)                                \
@@ -199,6 +200,16 @@ struct SimplifiedOperatorGlobalCache final {
   PURE_OP_LIST(PURE)
 #undef PURE
 
+#define NO_THROW(Name, properties, input_count)                               \
+  struct Name##Operator final : public Operator {                             \
+    Name##Operator()                                                          \
+        : Operator(IrOpcode::k##Name, Operator::kNoThrow | properties, #Name, \
+                   input_count, 1, 1, 1, 1, 0) {}                             \
+  };                                                                          \
+  Name##Operator k##Name;
+  NO_THROW_OP_LIST(NO_THROW)
+#undef NO_THROW
+
 #define BUFFER_ACCESS(Type, type, TYPE, ctype, size)                          \
   struct LoadBuffer##Type##Operator final : public Operator1<BufferAccess> {  \
     LoadBuffer##Type##Operator()                                              \
@@ -229,10 +240,11 @@ SimplifiedOperatorBuilder::SimplifiedOperatorBuilder(Zone* zone)
     : cache_(kCache.Get()), zone_(zone) {}
 
 
-#define PURE(Name, properties, input_count) \
+#define GET_FROM_CACHE(Name, properties, input_count) \
   const Operator* SimplifiedOperatorBuilder::Name() { return &cache_.k##Name; }
-PURE_OP_LIST(PURE)
-#undef PURE
+PURE_OP_LIST(GET_FROM_CACHE)
+NO_THROW_OP_LIST(GET_FROM_CACHE)
+#undef GET_FROM_CACHE
 
 
 const Operator* SimplifiedOperatorBuilder::ReferenceEqual(Type* type) {

diff --git a/test/cctest/compiler/test-js-typed-lowering.cc b/test/cctest/compiler/test-js-typed-lowering.cc
index 603b62d..bac511d 100644 (file)
--- a/test/cctest/compiler/test-js-typed-lowering.cc
+++ b/test/cctest/compiler/test-js-typed-lowering.cc
@@ -105,14 +105,12 @@ class JSTypedLoweringTester : public HandleAndZoneScope {
 
   Node* control() { return start(); }
 
-  void CheckPureBinop(IrOpcode::Value expected, Node* node) {
+  void CheckBinop(IrOpcode::Value expected, Node* node) {
     CHECK_EQ(expected, node->opcode());
-    CHECK_EQ(2, node->InputCount());  // should not have context, effect, etc.
   }
 
-  void CheckPureBinop(const Operator* expected, Node* node) {
+  void CheckBinop(const Operator* expected, Node* node) {
     CHECK_EQ(expected->opcode(), node->op()->opcode());
-    CHECK_EQ(2, node->InputCount());  // should not have context, effect, etc.
   }
 
   Node* ReduceUnop(const Operator* op, Type* input_type) {
@@ -249,7 +247,7 @@ TEST_WITH_STRONG(StringBinops) {
       Node* add = R.Binop(R.javascript.Add(language_mode), p0, p1);
       Node* r = R.reduce(add);
 
-      R.CheckPureBinop(IrOpcode::kStringAdd, r);
+      R.CheckBinop(IrOpcode::kStringAdd, r);
       CHECK_EQ(p0, r->InputAt(0));
       CHECK_EQ(p1, r->InputAt(1));
     }
@@ -266,7 +264,7 @@ TEST_WITH_STRONG(AddNumber1) {
     Node* add = R.Binop(R.javascript.Add(language_mode), p0, p1);
     Node* r = R.reduce(add);
 
-    R.CheckPureBinop(IrOpcode::kNumberAdd, r);
+    R.CheckBinop(IrOpcode::kNumberAdd, r);
     CHECK_EQ(p0, r->InputAt(0));
     CHECK_EQ(p1, r->InputAt(1));
   }
@@ -293,7 +291,7 @@ TEST_WITH_STRONG(NumberBinops) {
         Node* add = R.Binop(ops[k], p0, p1);
         Node* r = R.reduce(add);
 
-        R.CheckPureBinop(ops[k + 1], r);
+        R.CheckBinop(ops[k + 1], r);
         CHECK_EQ(p0, r->InputAt(0));
         CHECK_EQ(p1, r->InputAt(1));
       }
@@ -363,7 +361,7 @@ TEST(Int32BitwiseShifts) {
         Node* add = R.Binop(R.ops[k], p0, p1);
         Node* r = R.reduce(add);
 
-        R.CheckPureBinop(R.ops[k + 1], r);
+        R.CheckBinop(R.ops[k + 1], r);
         Node* r0 = r->InputAt(0);
         Node* r1 = r->InputAt(1);
 
@@ -421,7 +419,7 @@ TEST(Int32BitwiseBinops) {
         Node* add = R.Binop(R.ops[k], p0, p1);
         Node* r = R.reduce(add);
 
-        R.CheckPureBinop(R.ops[k + 1], r);
+        R.CheckBinop(R.ops[k + 1], r);
 
         CheckToI32(p0, r->InputAt(0), R.signedness[k]);
         CheckToI32(p1, r->InputAt(1), R.signedness[k + 1]);
@@ -614,7 +612,7 @@ TEST_WITH_STRONG(StringComparison) {
         Node* cmp = R.Binop(ops[k], p0, p1);
         Node* r = R.reduce(cmp);
 
-        R.CheckPureBinop(ops[k + 1], r);
+        R.CheckBinop(ops[k + 1], r);
         if (k >= 4) {
           // GreaterThan and GreaterThanOrEqual commute the inputs
           // and use the LessThan and LessThanOrEqual operators.
@@ -662,7 +660,7 @@ TEST_WITH_STRONG(NumberComparison) {
     Node* cmp = R.Binop(ops[k], p0, p1);
     Node* r = R.reduce(cmp);
 
-    R.CheckPureBinop(ops[k + 1], r);
+    R.CheckBinop(ops[k + 1], r);
     if (k >= 4) {
       // GreaterThan and GreaterThanOrEqual commute the inputs
       // and use the LessThan and LessThanOrEqual operators.
@@ -692,13 +690,13 @@ TEST_WITH_STRONG(MixedComparison1) {
         Node* cmp = R.Binop(less_than, p0, p1);
         Node* r = R.reduce(cmp);
         if (types[i]->Is(Type::String()) && types[j]->Is(Type::String())) {
-          R.CheckPureBinop(R.simplified.StringLessThan(), r);
+          R.CheckBinop(R.simplified.StringLessThan(), r);
         } else if ((types[i]->Is(Type::Number()) &&
                     types[j]->Is(Type::Number())) ||
                    (!is_strong(language_mode) &&
                     (!types[i]->Maybe(Type::String()) ||
                      !types[j]->Maybe(Type::String())))) {
-          R.CheckPureBinop(R.simplified.NumberLessThan(), r);
+          R.CheckBinop(R.simplified.NumberLessThan(), r);
         } else {
           // No reduction of mixed types.
           CHECK_EQ(r->op(), less_than);
@@ -839,7 +837,7 @@ void CheckEqualityReduction(JSTypedLoweringTester* R, bool strict, Node* l,
           strict ? R->javascript.StrictEqual() : R->javascript.Equal();
       Node* eq = R->Binop(op, p0, p1);
       Node* r = R->reduce(eq);
-      R->CheckPureBinop(expected, r);
+      R->CheckBinop(expected, r);
     }
 
     {
@@ -849,7 +847,7 @@ void CheckEqualityReduction(JSTypedLoweringTester* R, bool strict, Node* l,
       Node* n = R->reduce(ne);
       CHECK_EQ(IrOpcode::kBooleanNot, n->opcode());
       Node* r = n->InputAt(0);
-      R->CheckPureBinop(expected, r);
+      R->CheckBinop(expected, r);
     }
   }
 }
@@ -920,7 +918,7 @@ TEST_WITH_STRONG(RemovePureNumberBinopEffects) {
     BinopEffectsTester B(ops[j], Type::Number(), Type::Number());
     CHECK_EQ(ops[j + 1]->opcode(), B.result->op()->opcode());
 
-    B.R.CheckPureBinop(B.result->opcode(), B.result);
+    B.R.CheckBinop(B.result->opcode(), B.result);
 
     B.CheckNoOp(0);
     B.CheckNoOp(1);
@@ -1060,7 +1058,7 @@ TEST(Int32BinopEffects) {
     BinopEffectsTester B(R.ops[j], I32Type(signed_left), I32Type(signed_right));
     CHECK_EQ(R.ops[j + 1]->opcode(), B.result->op()->opcode());
 
-    B.R.CheckPureBinop(B.result->opcode(), B.result);
+    B.R.CheckBinop(B.result->opcode(), B.result);
 
     B.CheckNoOp(0);
     B.CheckNoOp(1);
@@ -1073,7 +1071,7 @@ TEST(Int32BinopEffects) {
     BinopEffectsTester B(R.ops[j], Type::Number(), Type::Number());
     CHECK_EQ(R.ops[j + 1]->opcode(), B.result->op()->opcode());
 
-    B.R.CheckPureBinop(B.result->opcode(), B.result);
+    B.R.CheckBinop(B.result->opcode(), B.result);
 
     B.CheckConvertedInput(NumberToI32(signed_left), 0, false);
     B.CheckConvertedInput(NumberToI32(signed_right), 1, false);
@@ -1085,7 +1083,7 @@ TEST(Int32BinopEffects) {
     bool signed_left = R.signedness[j], signed_right = R.signedness[j + 1];
     BinopEffectsTester B(R.ops[j], Type::Number(), Type::Primitive());
 
-    B.R.CheckPureBinop(B.result->opcode(), B.result);
+    B.R.CheckBinop(B.result->opcode(), B.result);
 
     Node* i0 = B.CheckConvertedInput(NumberToI32(signed_left), 0, false);
     Node* i1 = B.CheckConvertedInput(NumberToI32(signed_right), 1, false);
@@ -1102,7 +1100,7 @@ TEST(Int32BinopEffects) {
     bool signed_left = R.signedness[j], signed_right = R.signedness[j + 1];
     BinopEffectsTester B(R.ops[j], Type::Primitive(), Type::Number());
 
-    B.R.CheckPureBinop(B.result->opcode(), B.result);
+    B.R.CheckBinop(B.result->opcode(), B.result);
 
     Node* i0 = B.CheckConvertedInput(NumberToI32(signed_left), 0, false);
     Node* i1 = B.CheckConvertedInput(NumberToI32(signed_right), 1, false);
@@ -1119,7 +1117,7 @@ TEST(Int32BinopEffects) {
     bool signed_left = R.signedness[j], signed_right = R.signedness[j + 1];
     BinopEffectsTester B(R.ops[j], Type::Primitive(), Type::Primitive());
 
-    B.R.CheckPureBinop(B.result->opcode(), B.result);
+    B.R.CheckBinop(B.result->opcode(), B.result);
 
     Node* i0 = B.CheckConvertedInput(NumberToI32(signed_left), 0, false);
     Node* i1 = B.CheckConvertedInput(NumberToI32(signed_right), 1, false);
@@ -1251,7 +1249,7 @@ TEST_WITH_STRONG(Int32Comparisons) {
         } else {
           expected = ops[o].num_op;
         }
-        R.CheckPureBinop(expected, r);
+        R.CheckBinop(expected, r);
         if (ops[o].commute) {
           CHECK_EQ(p1, r->InputAt(0));
           CHECK_EQ(p0, r->InputAt(1));

diff --git a/test/cctest/compiler/test-simplified-lowering.cc b/test/cctest/compiler/test-simplified-lowering.cc
index 934f958..54ffe85 100644 (file)
--- a/test/cctest/compiler/test-simplified-lowering.cc
+++ b/test/cctest/compiler/test-simplified-lowering.cc
@@ -698,6 +698,13 @@ class TestingGraph : public HandleAndZoneScope, public GraphAndBuilders {
     CHECK_EQ(expected, node->opcode());
   }
 
+  void CheckLoweringStringBinop(IrOpcode::Value expected, const Operator* op) {
+    Node* node = Return(
+        graph()->NewNode(op, p0, p1, graph()->start(), graph()->start()));
+    Lower();
+    CHECK_EQ(expected, node->opcode());
+  }
+
   void CheckLoweringTruncatedBinop(IrOpcode::Value expected, const Operator* op,
                                    const Operator* trunc) {
     Node* node = graph()->NewNode(op, p0, p1);
@@ -1173,9 +1180,10 @@ TEST(LowerStringOps_to_call_and_compare) {
         static_cast<IrOpcode::Value>(t.machine()->IntLessThan()->opcode());
     IrOpcode::Value compare_le = static_cast<IrOpcode::Value>(
         t.machine()->IntLessThanOrEqual()->opcode());
-    t.CheckLoweringBinop(compare_eq, t.simplified()->StringEqual());
-    t.CheckLoweringBinop(compare_lt, t.simplified()->StringLessThan());
-    t.CheckLoweringBinop(compare_le, t.simplified()->StringLessThanOrEqual());
+    t.CheckLoweringStringBinop(compare_eq, t.simplified()->StringEqual());
+    t.CheckLoweringStringBinop(compare_lt, t.simplified()->StringLessThan());
+    t.CheckLoweringStringBinop(compare_le,
+                               t.simplified()->StringLessThanOrEqual());
   }
 
 

diff --git a/test/unittests/compiler/simplified-operator-unittest.cc b/test/unittests/compiler/simplified-operator-unittest.cc
index f4c74a5..7e4329e 100644 (file)
--- a/test/unittests/compiler/simplified-operator-unittest.cc
+++ b/test/unittests/compiler/simplified-operator-unittest.cc
@@ -51,9 +51,6 @@ const PureOperator kPureOperators[] = {
     PURE(NumberToInt32, Operator::kNoProperties, 1),
     PURE(NumberToUint32, Operator::kNoProperties, 1),
     PURE(PlainPrimitiveToNumber, Operator::kNoProperties, 1),
-    PURE(StringEqual, Operator::kCommutative, 2),
-    PURE(StringLessThan, Operator::kNoProperties, 2),
-    PURE(StringLessThanOrEqual, Operator::kNoProperties, 2),
     PURE(ChangeTaggedToInt32, Operator::kNoProperties, 1),
     PURE(ChangeTaggedToUint32, Operator::kNoProperties, 1),
     PURE(ChangeTaggedToFloat64, Operator::kNoProperties, 1),