From d06930fc4b19a759972950e81d55eb93d9a572c0 Mon Sep 17 00:00:00 2001 From: jarin Date: Tue, 29 Sep 2015 06:51:25 -0700 Subject: [PATCH] [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} --- src/compiler/js-generic-lowering.cc | 20 +++--- src/compiler/js-operator.cc | 10 +-- src/compiler/js-typed-lowering.cc | 12 +++- .../cctest/compiler/test-js-typed-lowering.cc | 14 ++-- .../js-intrinsic-lowering-unittest.cc | 4 +- .../compiler/js-operator-unittest.cc | 10 +-- .../compiler/js-typed-lowering-unittest.cc | 64 ++++++++++--------- 7 files changed, 72 insertions(+), 62 deletions(-) diff --git a/src/compiler/js-generic-lowering.cc b/src/compiler/js-generic-lowering.cc index 4937b1dea3..2d4210dff4 100644 --- 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 e7de5ee585..d708034687 100644 --- 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 5a0a000176..6c080d9085 100644 --- 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 2378622b9d..603b62d9cf 100644 --- 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 99f4574960..9d5b649471 100644 --- 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 0f33ddea8d..1ea5a652d1 100644 --- 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 e061e326f0..da964db23a 100644 --- 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)); }