[turbofan] Make Strict(Not)Equal, TypeOf, ToBoolean, UnaryNot effectful.

This is necessary because these operators can read heap (equality can actually write heap when flattening strings).

BUG=v8:4446
LOG=n

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

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

diff --git a/src/compiler/js-generic-lowering.cc b/src/compiler/js-generic-lowering.cc
index 4937b1d..2d4210d 100644 (file)
--- a/src/compiler/js-generic-lowering.cc
+++ b/src/compiler/js-generic-lowering.cc
@@ -149,17 +149,19 @@ void JSGenericLowering::ReplaceWithCompareIC(Node* node, Token::Value token,
   inputs.push_back(NodeProperties::GetValueInput(node, 0));
   inputs.push_back(NodeProperties::GetValueInput(node, 1));
   inputs.push_back(NodeProperties::GetContextInput(node));
-  if (node->op()->HasProperty(Operator::kPure)) {
-    // A pure (strict) comparison doesn't have an effect, control or frame
-    // state.  But for the graph, we need to add control and effect inputs.
-    DCHECK(OperatorProperties::GetFrameStateInputCount(node->op()) == 0);
-    inputs.push_back(graph()->start());
-    inputs.push_back(graph()->start());
-  } else {
+  // Some comparisons (StrictEqual) don't have an effect, control or frame
+  // state inputs, so handle those cases here.
+  if (OperatorProperties::GetFrameStateInputCount(node->op()) > 0) {
     inputs.push_back(NodeProperties::GetFrameStateInput(node, 0));
-    inputs.push_back(NodeProperties::GetEffectInput(node));
-    inputs.push_back(NodeProperties::GetControlInput(node));
   }
+  Node* effect = (node->op()->EffectInputCount() > 0)
+                     ? NodeProperties::GetEffectInput(node)
+                     : graph()->start();
+  inputs.push_back(effect);
+  Node* control = (node->op()->ControlInputCount() > 0)
+                      ? NodeProperties::GetControlInput(node)
+                      : graph()->start();
+  inputs.push_back(control);
   CallDescriptor* desc_compare = Linkage::GetStubCallDescriptor(
       isolate(), zone(), callable.descriptor(), 0,
       CallDescriptor::kPatchableCallSiteWithNop | FlagsForNode(node),

diff --git a/src/compiler/js-operator.cc b/src/compiler/js-operator.cc
index e7de5ee..d708034 100644 (file)
--- a/src/compiler/js-operator.cc
+++ b/src/compiler/js-operator.cc
@@ -454,10 +454,10 @@ const CreateClosureParameters& CreateClosureParametersOf(const Operator* op) {
 #define CACHED_OP_LIST(V)                                 \
   V(Equal, Operator::kNoProperties, 2, 1)                 \
   V(NotEqual, Operator::kNoProperties, 2, 1)              \
-  V(StrictEqual, Operator::kPure, 2, 1)                   \
-  V(StrictNotEqual, Operator::kPure, 2, 1)                \
-  V(UnaryNot, Operator::kPure, 1, 1)                      \
-  V(ToBoolean, Operator::kPure, 1, 1)                     \
+  V(StrictEqual, Operator::kNoThrow, 2, 1)                \
+  V(StrictNotEqual, Operator::kNoThrow, 2, 1)             \
+  V(UnaryNot, Operator::kEliminatable, 1, 1)              \
+  V(ToBoolean, Operator::kEliminatable, 1, 1)             \
   V(ToNumber, Operator::kNoProperties, 1, 1)              \
   V(ToString, Operator::kNoProperties, 1, 1)              \
   V(ToName, Operator::kNoProperties, 1, 1)                \
@@ -465,7 +465,7 @@ const CreateClosureParameters& CreateClosureParametersOf(const Operator* op) {
   V(Yield, Operator::kNoProperties, 1, 1)                 \
   V(Create, Operator::kEliminatable, 0, 1)                \
   V(HasProperty, Operator::kNoProperties, 2, 1)           \
-  V(TypeOf, Operator::kPure, 1, 1)                        \
+  V(TypeOf, Operator::kEliminatable, 1, 1)                \
   V(InstanceOf, Operator::kNoProperties, 2, 1)            \
   V(ForInDone, Operator::kPure, 2, 1)                     \
   V(ForInNext, Operator::kNoProperties, 4, 1)             \

diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc
index 5a0a000..6c080d9 100644 (file)
--- a/src/compiler/js-typed-lowering.cc
+++ b/src/compiler/js-typed-lowering.cc
@@ -576,7 +576,7 @@ Reduction JSTypedLowering::ReduceJSStrictEqual(Node* node, bool invert) {
     // x === x is always true if x != NaN
     if (!r.left_type()->Maybe(Type::NaN())) {
       Node* replacement = jsgraph()->BooleanConstant(!invert);
-      Replace(node, replacement);
+      ReplaceWithValue(node, replacement);
       return Replace(replacement);
     }
   }
@@ -585,7 +585,7 @@ Reduction JSTypedLowering::ReduceJSStrictEqual(Node* node, bool invert) {
     // empty type intersection means the values cannot be strictly equal.
     if (!r.left_type()->Maybe(r.right_type())) {
       Node* replacement = jsgraph()->BooleanConstant(invert);
-      Replace(node, replacement);
+      ReplaceWithValue(node, replacement);
       return Replace(replacement);
     }
   }
@@ -629,12 +629,15 @@ Reduction JSTypedLowering::ReduceJSUnaryNot(Node* node) {
   Type* const input_type = NodeProperties::GetType(input);
   if (input_type->Is(Type::Boolean())) {
     // JSUnaryNot(x:boolean) => BooleanNot(x)
+    RelaxEffectsAndControls(node);
     node->TrimInputCount(1);
     NodeProperties::ChangeOp(node, simplified()->BooleanNot());
     return Changed(node);
   } else if (input_type->Is(Type::OrderedNumber())) {
     // JSUnaryNot(x:number) => NumberEqual(x,#0)
+    RelaxEffectsAndControls(node);
     node->ReplaceInput(1, jsgraph()->ZeroConstant());
+    node->TrimInputCount(2);
     NodeProperties::ChangeOp(node, simplified()->NumberEqual());
     return Changed(node);
   } else if (input_type->Is(Type::String())) {
@@ -647,6 +650,7 @@ Reduction JSTypedLowering::ReduceJSUnaryNot(Node* node) {
     ReplaceWithValue(node, node, length);
     node->ReplaceInput(0, length);
     node->ReplaceInput(1, jsgraph()->ZeroConstant());
+    node->TrimInputCount(2);
     NodeProperties::ChangeOp(node, simplified()->NumberEqual());
     return Changed(node);
   }
@@ -659,9 +663,11 @@ Reduction JSTypedLowering::ReduceJSToBoolean(Node* node) {
   Type* const input_type = NodeProperties::GetType(input);
   if (input_type->Is(Type::Boolean())) {
     // JSToBoolean(x:boolean) => x
+    ReplaceWithValue(node, input);
     return Replace(input);
   } else if (input_type->Is(Type::OrderedNumber())) {
     // JSToBoolean(x:ordered-number) => BooleanNot(NumberEqual(x,#0))
+    RelaxEffectsAndControls(node);
     node->ReplaceInput(0, graph()->NewNode(simplified()->NumberEqual(), input,
                                            jsgraph()->ZeroConstant()));
     node->TrimInputCount(1);
@@ -674,8 +680,10 @@ Reduction JSTypedLowering::ReduceJSToBoolean(Node* node) {
     // chain) because we assume String::length to be immutable.
     Node* length = graph()->NewNode(simplified()->LoadField(access), input,
                                     graph()->start(), graph()->start());
+    ReplaceWithValue(node, node, length);
     node->ReplaceInput(0, jsgraph()->ZeroConstant());
     node->ReplaceInput(1, length);
+    node->TrimInputCount(2);
     NodeProperties::ChangeOp(node, simplified()->NumberLessThan());
     return Changed(node);
   }

diff --git a/test/cctest/compiler/test-js-typed-lowering.cc b/test/cctest/compiler/test-js-typed-lowering.cc
index 2378622..603b62d 100644 (file)
--- a/test/cctest/compiler/test-js-typed-lowering.cc
+++ b/test/cctest/compiler/test-js-typed-lowering.cc
@@ -835,19 +835,17 @@ void CheckEqualityReduction(JSTypedLoweringTester* R, bool strict, Node* l,
     Node* p1 = j == 1 ? l : r;
 
     {
-      Node* eq = strict
-                     ? R->graph.NewNode(R->javascript.StrictEqual(), p0, p1,
-                                        R->context())
-                     : R->Binop(R->javascript.Equal(), p0, p1);
+      const Operator* op =
+          strict ? R->javascript.StrictEqual() : R->javascript.Equal();
+      Node* eq = R->Binop(op, p0, p1);
       Node* r = R->reduce(eq);
       R->CheckPureBinop(expected, r);
     }
 
     {
-      Node* ne = strict
-                     ? R->graph.NewNode(R->javascript.StrictNotEqual(), p0, p1,
-                                        R->context())
-                     : R->Binop(R->javascript.NotEqual(), p0, p1);
+      const Operator* op =
+          strict ? R->javascript.StrictNotEqual() : R->javascript.NotEqual();
+      Node* ne = R->Binop(op, p0, p1);
       Node* n = R->reduce(ne);
       CHECK_EQ(IrOpcode::kBooleanNot, n->opcode());
       Node* r = n->InputAt(0);

diff --git a/test/unittests/compiler/js-intrinsic-lowering-unittest.cc b/test/unittests/compiler/js-intrinsic-lowering-unittest.cc
index 99f4574..9d5b649 100644 (file)
--- a/test/unittests/compiler/js-intrinsic-lowering-unittest.cc
+++ b/test/unittests/compiler/js-intrinsic-lowering-unittest.cc
@@ -310,7 +310,7 @@ TEST_F(JSIntrinsicLoweringTest, Likely) {
       graph()->NewNode(javascript()->CallRuntime(Runtime::kInlineLikely, 1),
                        input, context, effect, control);
   Node* const to_boolean =
-      graph()->NewNode(javascript()->ToBoolean(), likely, context);
+      graph()->NewNode(javascript()->ToBoolean(), likely, context, effect);
   Diamond d(graph(), common(), to_boolean);
   graph()->SetEnd(graph()->NewNode(common()->End(1), d.merge));
 
@@ -405,7 +405,7 @@ TEST_F(JSIntrinsicLoweringTest, Unlikely) {
       graph()->NewNode(javascript()->CallRuntime(Runtime::kInlineUnlikely, 1),
                        input, context, effect, control);
   Node* const to_boolean =
-      graph()->NewNode(javascript()->ToBoolean(), unlikely, context);
+      graph()->NewNode(javascript()->ToBoolean(), unlikely, context, effect);
   Diamond d(graph(), common(), to_boolean);
   graph()->SetEnd(graph()->NewNode(common()->End(1), d.merge));
 

diff --git a/test/unittests/compiler/js-operator-unittest.cc b/test/unittests/compiler/js-operator-unittest.cc
index 0f33dde..1ea5a65 100644 (file)
--- a/test/unittests/compiler/js-operator-unittest.cc
+++ b/test/unittests/compiler/js-operator-unittest.cc
@@ -69,10 +69,10 @@ const SharedOperator kSharedOperators[] = {
   }
     SHARED(Equal, Operator::kNoProperties, 2, 1, 1, 1, 1, 1, 2),
     SHARED(NotEqual, Operator::kNoProperties, 2, 1, 1, 1, 1, 1, 2),
-    SHARED(StrictEqual, Operator::kPure, 2, 0, 0, 0, 1, 0, 0),
-    SHARED(StrictNotEqual, Operator::kPure, 2, 0, 0, 0, 1, 0, 0),
-    SHARED(UnaryNot, Operator::kPure, 1, 0, 0, 0, 1, 0, 0),
-    SHARED(ToBoolean, Operator::kPure, 1, 0, 0, 0, 1, 0, 0),
+    SHARED(StrictEqual, Operator::kNoThrow, 2, 0, 1, 1, 1, 1, 0),
+    SHARED(StrictNotEqual, Operator::kNoThrow, 2, 0, 1, 1, 1, 1, 0),
+    SHARED(UnaryNot, Operator::kEliminatable, 1, 0, 1, 0, 1, 1, 0),
+    SHARED(ToBoolean, Operator::kEliminatable, 1, 0, 1, 0, 1, 1, 0),
     SHARED(ToNumber, Operator::kNoProperties, 1, 1, 1, 1, 1, 1, 2),
     SHARED(ToString, Operator::kNoProperties, 1, 0, 1, 1, 1, 1, 2),
     SHARED(ToName, Operator::kNoProperties, 1, 1, 1, 1, 1, 1, 2),
@@ -80,7 +80,7 @@ const SharedOperator kSharedOperators[] = {
     SHARED(Yield, Operator::kNoProperties, 1, 0, 1, 1, 1, 1, 2),
     SHARED(Create, Operator::kEliminatable, 0, 0, 1, 0, 1, 1, 0),
     SHARED(HasProperty, Operator::kNoProperties, 2, 1, 1, 1, 1, 1, 2),
-    SHARED(TypeOf, Operator::kPure, 1, 0, 0, 0, 1, 0, 0),
+    SHARED(TypeOf, Operator::kEliminatable, 1, 0, 1, 0, 1, 1, 0),
     SHARED(InstanceOf, Operator::kNoProperties, 2, 1, 1, 1, 1, 1, 2),
     SHARED(CreateFunctionContext, Operator::kNoProperties, 1, 0, 1, 1, 1, 1, 2),
     SHARED(CreateWithContext, Operator::kNoProperties, 2, 1, 1, 1, 1, 1, 2),

diff --git a/test/unittests/compiler/js-typed-lowering-unittest.cc b/test/unittests/compiler/js-typed-lowering-unittest.cc
index e061e32..da964db 100644 (file)
--- a/test/unittests/compiler/js-typed-lowering-unittest.cc
+++ b/test/unittests/compiler/js-typed-lowering-unittest.cc
@@ -112,8 +112,8 @@ class JSTypedLoweringTest : public TypedGraphTest {
 TEST_F(JSTypedLoweringTest, JSUnaryNotWithBoolean) {
   Node* input = Parameter(Type::Boolean(), 0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->UnaryNot(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->UnaryNot(), input,
+                                        context, graph()->start()));
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsBooleanNot(input));
 }
@@ -122,8 +122,8 @@ TEST_F(JSTypedLoweringTest, JSUnaryNotWithBoolean) {
 TEST_F(JSTypedLoweringTest, JSUnaryNotWithOrderedNumber) {
   Node* input = Parameter(Type::OrderedNumber(), 0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->UnaryNot(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->UnaryNot(), input,
+                                        context, graph()->start()));
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsNumberEqual(input, IsNumberConstant(0)));
 }
@@ -151,8 +151,8 @@ TEST_F(JSTypedLoweringTest, JSUnaryNotWithFalsish) {
           zone()),
       0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->UnaryNot(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->UnaryNot(), input,
+                                        context, graph()->start()));
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsTrueConstant());
 }
@@ -166,8 +166,8 @@ TEST_F(JSTypedLoweringTest, JSUnaryNotWithTruish) {
           zone()),
       0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->UnaryNot(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->UnaryNot(), input,
+                                        context, graph()->start()));
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsFalseConstant());
 }
@@ -176,8 +176,8 @@ TEST_F(JSTypedLoweringTest, JSUnaryNotWithTruish) {
 TEST_F(JSTypedLoweringTest, JSUnaryNotWithNonZeroPlainNumber) {
   Node* input = Parameter(Type::Range(1.0, 42.0, zone()), 0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->UnaryNot(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->UnaryNot(), input,
+                                        context, graph()->start()));
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsFalseConstant());
 }
@@ -186,8 +186,8 @@ TEST_F(JSTypedLoweringTest, JSUnaryNotWithNonZeroPlainNumber) {
 TEST_F(JSTypedLoweringTest, JSUnaryNotWithString) {
   Node* input = Parameter(Type::String(), 0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->UnaryNot(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->UnaryNot(), input,
+                                        context, graph()->start()));
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(
       r.replacement(),
@@ -200,8 +200,8 @@ TEST_F(JSTypedLoweringTest, JSUnaryNotWithString) {
 TEST_F(JSTypedLoweringTest, JSUnaryNotWithAny) {
   Node* input = Parameter(Type::Any(), 0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->UnaryNot(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->UnaryNot(), input,
+                                        context, graph()->start()));
   ASSERT_FALSE(r.Changed());
 }
 
@@ -307,8 +307,8 @@ TEST_F(JSTypedLoweringTest, ParameterWithUndefined) {
 TEST_F(JSTypedLoweringTest, JSToBooleanWithBoolean) {
   Node* input = Parameter(Type::Boolean(), 0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->ToBoolean(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->ToBoolean(), input,
+                                        context, graph()->start()));
   ASSERT_TRUE(r.Changed());
   EXPECT_EQ(input, r.replacement());
 }
@@ -336,8 +336,8 @@ TEST_F(JSTypedLoweringTest, JSToBooleanWithFalsish) {
           zone()),
       0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->ToBoolean(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->ToBoolean(), input,
+                                        context, graph()->start()));
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsFalseConstant());
 }
@@ -351,8 +351,8 @@ TEST_F(JSTypedLoweringTest, JSToBooleanWithTruish) {
           zone()),
       0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->ToBoolean(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->ToBoolean(), input,
+                                        context, graph()->start()));
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsTrueConstant());
 }
@@ -361,8 +361,8 @@ TEST_F(JSTypedLoweringTest, JSToBooleanWithTruish) {
 TEST_F(JSTypedLoweringTest, JSToBooleanWithNonZeroPlainNumber) {
   Node* input = Parameter(Type::Range(1, V8_INFINITY, zone()), 0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->ToBoolean(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->ToBoolean(), input,
+                                        context, graph()->start()));
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsTrueConstant());
 }
@@ -371,8 +371,8 @@ TEST_F(JSTypedLoweringTest, JSToBooleanWithNonZeroPlainNumber) {
 TEST_F(JSTypedLoweringTest, JSToBooleanWithOrderedNumber) {
   Node* input = Parameter(Type::OrderedNumber(), 0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->ToBoolean(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->ToBoolean(), input,
+                                        context, graph()->start()));
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(),
               IsBooleanNot(IsNumberEqual(input, IsNumberConstant(0.0))));
@@ -382,8 +382,8 @@ TEST_F(JSTypedLoweringTest, JSToBooleanWithOrderedNumber) {
 TEST_F(JSTypedLoweringTest, JSToBooleanWithString) {
   Node* input = Parameter(Type::String(), 0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->ToBoolean(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->ToBoolean(), input,
+                                        context, graph()->start()));
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(
       r.replacement(),
@@ -396,8 +396,8 @@ TEST_F(JSTypedLoweringTest, JSToBooleanWithString) {
 TEST_F(JSTypedLoweringTest, JSToBooleanWithAny) {
   Node* input = Parameter(Type::Any(), 0);
   Node* context = Parameter(Type::Any(), 1);
-  Reduction r =
-      Reduce(graph()->NewNode(javascript()->ToBoolean(), input, context));
+  Reduction r = Reduce(graph()->NewNode(javascript()->ToBoolean(), input,
+                                        context, graph()->start()));
   ASSERT_FALSE(r.Changed());
 }
 
@@ -429,8 +429,9 @@ TEST_F(JSTypedLoweringTest, JSStrictEqualWithTheHole) {
   Node* const context = UndefinedConstant();
   TRACED_FOREACH(Type*, type, kJSTypes) {
     Node* const lhs = Parameter(type);
-    Reduction r = Reduce(
-        graph()->NewNode(javascript()->StrictEqual(), lhs, the_hole, context));
+    Reduction r =
+        Reduce(graph()->NewNode(javascript()->StrictEqual(), lhs, the_hole,
+                                context, graph()->start(), graph()->start()));
     ASSERT_TRUE(r.Changed());
     EXPECT_THAT(r.replacement(), IsFalseConstant());
   }
@@ -442,7 +443,8 @@ TEST_F(JSTypedLoweringTest, JSStrictEqualWithUnique) {
   Node* const rhs = Parameter(Type::Unique(), 1);
   Node* const context = Parameter(Type::Any(), 2);
   Reduction r =
-      Reduce(graph()->NewNode(javascript()->StrictEqual(), lhs, rhs, context));
+      Reduce(graph()->NewNode(javascript()->StrictEqual(), lhs, rhs, context,
+                              graph()->start(), graph()->start()));
   ASSERT_TRUE(r.Changed());
   EXPECT_THAT(r.replacement(), IsReferenceEqual(Type::Unique(), lhs, rhs));
 }