Commit 2620c426 authored by mstarzinger's avatar mstarzinger Committed by Commit bot

[turbofan] Remove eager frame state from add and subtract.

This removes the frame state input representing the before-state from
nodes having the {JSAdd} or the {JSSubtract} operator. Lowering that
inserts number conversions of the inputs has to be disabled when
deoptimization is enabled, because the frame state layout is no longer
known.

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

Review-Url: https://codereview.chromium.org/2125593002
Cr-Commit-Position: refs/heads/master@{#37522}
parent 8465244e
...@@ -455,7 +455,9 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) { ...@@ -455,7 +455,9 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
r.ConvertInputsToNumber(); r.ConvertInputsToNumber();
return r.ChangeToPureOperator(simplified()->NumberAdd(), Type::Number()); return r.ChangeToPureOperator(simplified()->NumberAdd(), Type::Number());
} }
if (r.NeitherInputCanBe(Type::StringOrReceiver())) { if ((r.BothInputsAre(Type::PlainPrimitive()) ||
!(flags() & kDeoptimizationEnabled)) &&
r.NeitherInputCanBe(Type::StringOrReceiver())) {
// JSAdd(x:-string, y:-string) => NumberAdd(ToNumber(x), ToNumber(y)) // JSAdd(x:-string, y:-string) => NumberAdd(ToNumber(x), ToNumber(y))
r.ConvertInputsToNumber(); r.ConvertInputsToNumber();
return r.ChangeToPureOperator(simplified()->NumberAdd(), Type::Number()); return r.ChangeToPureOperator(simplified()->NumberAdd(), Type::Number());
...@@ -474,8 +476,7 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) { ...@@ -474,8 +476,7 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
CallDescriptor const* const desc = Linkage::GetStubCallDescriptor( CallDescriptor const* const desc = Linkage::GetStubCallDescriptor(
isolate(), graph()->zone(), callable.descriptor(), 0, isolate(), graph()->zone(), callable.descriptor(), 0,
CallDescriptor::kNeedsFrameState, node->op()->properties()); CallDescriptor::kNeedsFrameState, node->op()->properties());
DCHECK_EQ(2, OperatorProperties::GetFrameStateInputCount(node->op())); DCHECK_EQ(1, OperatorProperties::GetFrameStateInputCount(node->op()));
node->RemoveInput(NodeProperties::FirstFrameStateIndex(node) + 1);
node->InsertInput(graph()->zone(), 0, node->InsertInput(graph()->zone(), 0,
jsgraph()->HeapConstant(callable.code())); jsgraph()->HeapConstant(callable.code()));
NodeProperties::ChangeOp(node, common()->Call(desc)); NodeProperties::ChangeOp(node, common()->Call(desc));
...@@ -518,8 +519,16 @@ Reduction JSTypedLowering::ReduceJSSubtract(Node* node) { ...@@ -518,8 +519,16 @@ Reduction JSTypedLowering::ReduceJSSubtract(Node* node) {
return r.ChangeToSpeculativeOperator( return r.ChangeToSpeculativeOperator(
simplified()->SpeculativeNumberSubtract(feedback), Type::Number()); simplified()->SpeculativeNumberSubtract(feedback), Type::Number());
} }
r.ConvertInputsToNumber();
return r.ChangeToPureOperator(simplified()->NumberSubtract(), Type::Number()); // If deoptimization is enabled we rely on type feedback.
if (r.BothInputsAre(Type::PlainPrimitive()) ||
!(flags() & kDeoptimizationEnabled)) {
r.ConvertInputsToNumber();
return r.ChangeToPureOperator(simplified()->NumberSubtract(),
Type::Number());
}
return NoChange();
} }
Reduction JSTypedLowering::ReduceJSMultiply(Node* node) { Reduction JSTypedLowering::ReduceJSMultiply(Node* node) {
......
...@@ -36,6 +36,8 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) { ...@@ -36,6 +36,8 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) {
return 0; return 0;
// Binary operations // Binary operations
case IrOpcode::kJSAdd:
case IrOpcode::kJSSubtract:
case IrOpcode::kJSMultiply: case IrOpcode::kJSMultiply:
// Compare operations // Compare operations
...@@ -86,7 +88,6 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) { ...@@ -86,7 +88,6 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) {
// Binary operators that can deopt in the middle the operation (e.g., // Binary operators that can deopt in the middle the operation (e.g.,
// as a result of lazy deopt in ToNumber conversion) need a second frame // as a result of lazy deopt in ToNumber conversion) need a second frame
// state so that we can resume before the operation. // state so that we can resume before the operation.
case IrOpcode::kJSAdd:
case IrOpcode::kJSBitwiseAnd: case IrOpcode::kJSBitwiseAnd:
case IrOpcode::kJSBitwiseOr: case IrOpcode::kJSBitwiseOr:
case IrOpcode::kJSBitwiseXor: case IrOpcode::kJSBitwiseXor:
...@@ -95,7 +96,6 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) { ...@@ -95,7 +96,6 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) {
case IrOpcode::kJSShiftLeft: case IrOpcode::kJSShiftLeft:
case IrOpcode::kJSShiftRight: case IrOpcode::kJSShiftRight:
case IrOpcode::kJSShiftRightLogical: case IrOpcode::kJSShiftRightLogical:
case IrOpcode::kJSSubtract:
return 2; return 2;
// Compare operators that can deopt in the middle the operation (e.g., // Compare operators that can deopt in the middle the operation (e.g.,
......
...@@ -714,13 +714,11 @@ TEST(RemoveToNumberEffects) { ...@@ -714,13 +714,11 @@ TEST(RemoveToNumberEffects) {
effect_use = R.graph.NewNode(R.common.EffectPhi(1), ton, R.start()); effect_use = R.graph.NewNode(R.common.EffectPhi(1), ton, R.start());
case 3: case 3:
effect_use = R.graph.NewNode(R.javascript.Add(R.binop_hints), ton, ton, effect_use = R.graph.NewNode(R.javascript.Add(R.binop_hints), ton, ton,
R.context(), frame_state, frame_state, ton, R.context(), frame_state, ton, R.start());
R.start());
break; break;
case 4: case 4:
effect_use = R.graph.NewNode(R.javascript.Add(R.binop_hints), p0, p0, effect_use = R.graph.NewNode(R.javascript.Add(R.binop_hints), p0, p0,
R.context(), frame_state, frame_state, ton, R.context(), frame_state, ton, R.start());
R.start());
break; break;
case 5: case 5:
effect_use = R.graph.NewNode(R.common.Return(), p0, ton, R.start()); effect_use = R.graph.NewNode(R.common.Return(), p0, ton, R.start());
......
...@@ -826,19 +826,19 @@ TEST_F(JSTypedLoweringTest, JSAddWithString) { ...@@ -826,19 +826,19 @@ TEST_F(JSTypedLoweringTest, JSAddWithString) {
Node* lhs = Parameter(Type::String(), 0); Node* lhs = Parameter(Type::String(), 0);
Node* rhs = Parameter(Type::String(), 1); Node* rhs = Parameter(Type::String(), 1);
Node* context = Parameter(Type::Any(), 2); Node* context = Parameter(Type::Any(), 2);
Node* frame_state0 = EmptyFrameState(); Node* frame_state = EmptyFrameState();
Node* frame_state1 = EmptyFrameState();
Node* effect = graph()->start(); Node* effect = graph()->start();
Node* control = graph()->start(); Node* control = graph()->start();
Reduction r = Reduction r =
Reduce(graph()->NewNode(javascript()->Add(hints), lhs, rhs, context, Reduce(graph()->NewNode(javascript()->Add(hints), lhs, rhs, context,
frame_state0, frame_state1, effect, control)); frame_state, effect, control));
ASSERT_TRUE(r.Changed()); ASSERT_TRUE(r.Changed());
EXPECT_THAT(r.replacement(), EXPECT_THAT(r.replacement(),
IsCall(_, IsHeapConstant(CodeFactory::StringAdd( IsCall(_, IsHeapConstant(
isolate(), STRING_ADD_CHECK_NONE, CodeFactory::StringAdd(
NOT_TENURED).code()), isolate(), STRING_ADD_CHECK_NONE, NOT_TENURED)
lhs, rhs, context, frame_state0, effect, control)); .code()),
lhs, rhs, context, frame_state, effect, control));
} }
TEST_F(JSTypedLoweringTest, JSAddSmis) { TEST_F(JSTypedLoweringTest, JSAddSmis) {
...@@ -849,13 +849,12 @@ TEST_F(JSTypedLoweringTest, JSAddSmis) { ...@@ -849,13 +849,12 @@ TEST_F(JSTypedLoweringTest, JSAddSmis) {
Node* lhs = Parameter(Type::Number(), 0); Node* lhs = Parameter(Type::Number(), 0);
Node* rhs = Parameter(Type::Number(), 1); Node* rhs = Parameter(Type::Number(), 1);
Node* context = Parameter(Type::Any(), 2); Node* context = Parameter(Type::Any(), 2);
Node* frame_state0 = EmptyFrameState(); Node* frame_state = EmptyFrameState();
Node* frame_state1 = EmptyFrameState();
Node* effect = graph()->start(); Node* effect = graph()->start();
Node* control = graph()->start(); Node* control = graph()->start();
Reduction r = Reduction r =
Reduce(graph()->NewNode(javascript()->Add(hints), lhs, rhs, context, Reduce(graph()->NewNode(javascript()->Add(hints), lhs, rhs, context,
frame_state0, frame_state1, effect, control)); frame_state, effect, control));
ASSERT_TRUE(r.Changed()); ASSERT_TRUE(r.Changed());
EXPECT_THAT(r.replacement(), EXPECT_THAT(r.replacement(),
IsSpeculativeNumberAdd(BinaryOperationHints::kSignedSmall, lhs, IsSpeculativeNumberAdd(BinaryOperationHints::kSignedSmall, lhs,
...@@ -874,13 +873,12 @@ TEST_F(JSTypedLoweringTest, JSSubtractSmis) { ...@@ -874,13 +873,12 @@ TEST_F(JSTypedLoweringTest, JSSubtractSmis) {
Node* lhs = Parameter(Type::Number(), 0); Node* lhs = Parameter(Type::Number(), 0);
Node* rhs = Parameter(Type::Number(), 1); Node* rhs = Parameter(Type::Number(), 1);
Node* context = Parameter(Type::Any(), 2); Node* context = Parameter(Type::Any(), 2);
Node* frame_state0 = EmptyFrameState(); Node* frame_state = EmptyFrameState();
Node* frame_state1 = EmptyFrameState();
Node* effect = graph()->start(); Node* effect = graph()->start();
Node* control = graph()->start(); Node* control = graph()->start();
Reduction r = Reduce(graph()->NewNode(javascript()->Subtract(hints), lhs, Reduction r =
rhs, context, frame_state0, Reduce(graph()->NewNode(javascript()->Subtract(hints), lhs, rhs,
frame_state1, effect, control)); context, frame_state, effect, control));
ASSERT_TRUE(r.Changed()); ASSERT_TRUE(r.Changed());
EXPECT_THAT(r.replacement(), EXPECT_THAT(r.replacement(),
IsSpeculativeNumberSubtract(BinaryOperationHints::kSignedSmall, IsSpeculativeNumberSubtract(BinaryOperationHints::kSignedSmall,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment