Commit 277fac44 authored by mstarzinger's avatar mstarzinger Committed by Commit bot

[turbofan] Remove eager frame state from JSMultiply.

This removes the frame state input representing the before-state from
nodes having the {JSMultiply} 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/2111193002
Cr-Commit-Position: refs/heads/master@{#37517}
parent f310a829
...@@ -34,7 +34,7 @@ class JSBinopReduction final { ...@@ -34,7 +34,7 @@ class JSBinopReduction final {
} }
DCHECK_NE(0, node_->op()->ControlOutputCount()); DCHECK_NE(0, node_->op()->ControlOutputCount());
DCHECK_EQ(1, node_->op()->EffectOutputCount()); DCHECK_EQ(1, node_->op()->EffectOutputCount());
DCHECK_EQ(2, OperatorProperties::GetFrameStateInputCount(node_->op())); DCHECK_LE(1, OperatorProperties::GetFrameStateInputCount(node_->op()));
BinaryOperationHints hints = BinaryOperationHintsOf(node_->op()); BinaryOperationHints hints = BinaryOperationHintsOf(node_->op());
BinaryOperationHints::Hint combined = hints.combined(); BinaryOperationHints::Hint combined = hints.combined();
if (combined == BinaryOperationHints::kSignedSmall || if (combined == BinaryOperationHints::kSignedSmall ||
...@@ -155,7 +155,7 @@ class JSBinopReduction final { ...@@ -155,7 +155,7 @@ class JSBinopReduction final {
DCHECK_EQ(1, node_->op()->EffectOutputCount()); DCHECK_EQ(1, node_->op()->EffectOutputCount());
DCHECK_EQ(1, node_->op()->ControlInputCount()); DCHECK_EQ(1, node_->op()->ControlInputCount());
DCHECK_LT(1, node_->op()->ControlOutputCount()); DCHECK_LT(1, node_->op()->ControlOutputCount());
DCHECK_EQ(2, OperatorProperties::GetFrameStateInputCount(node_->op())); DCHECK_LE(1, OperatorProperties::GetFrameStateInputCount(node_->op()));
DCHECK_EQ(2, node_->op()->ValueInputCount()); DCHECK_EQ(2, node_->op()->ValueInputCount());
// Reconnect the control output to bypass the IfSuccess node and // Reconnect the control output to bypass the IfSuccess node and
...@@ -175,7 +175,9 @@ class JSBinopReduction final { ...@@ -175,7 +175,9 @@ class JSBinopReduction final {
} }
// Remove both bailout frame states and the context. // Remove both bailout frame states and the context.
node_->RemoveInput(NodeProperties::FirstFrameStateIndex(node_) + 1); if (OperatorProperties::GetFrameStateInputCount(node_->op()) == 2) {
node_->RemoveInput(NodeProperties::FirstFrameStateIndex(node_) + 1);
}
node_->RemoveInput(NodeProperties::FirstFrameStateIndex(node_)); node_->RemoveInput(NodeProperties::FirstFrameStateIndex(node_));
node_->RemoveInput(NodeProperties::FirstContextIndex(node_)); node_->RemoveInput(NodeProperties::FirstContextIndex(node_));
...@@ -230,6 +232,13 @@ class JSBinopReduction final { ...@@ -230,6 +232,13 @@ class JSBinopReduction final {
Node* node_; // The original node. Node* node_; // The original node.
Node* CreateFrameStateForLeftInput() { Node* CreateFrameStateForLeftInput() {
if (OperatorProperties::GetFrameStateInputCount(node_->op()) < 2) {
// Deoptimization is disabled => return dummy frame state instead.
Node* dummy_state = NodeProperties::GetFrameStateInput(node_, 0);
DCHECK(OpParameter<FrameStateInfo>(dummy_state).bailout_id().IsNone());
return dummy_state;
}
Node* frame_state = NodeProperties::GetFrameStateInput(node_, 1); Node* frame_state = NodeProperties::GetFrameStateInput(node_, 1);
FrameStateInfo state_info = OpParameter<FrameStateInfo>(frame_state); FrameStateInfo state_info = OpParameter<FrameStateInfo>(frame_state);
...@@ -261,6 +270,13 @@ class JSBinopReduction final { ...@@ -261,6 +270,13 @@ class JSBinopReduction final {
} }
Node* CreateFrameStateForRightInput(Node* converted_left) { Node* CreateFrameStateForRightInput(Node* converted_left) {
if (OperatorProperties::GetFrameStateInputCount(node_->op()) < 2) {
// Deoptimization is disabled => return dummy frame state instead.
Node* dummy_state = NodeProperties::GetFrameStateInput(node_, 0);
DCHECK(OpParameter<FrameStateInfo>(dummy_state).bailout_id().IsNone());
return dummy_state;
}
Node* frame_state = NodeProperties::GetFrameStateInput(node_, 1); Node* frame_state = NodeProperties::GetFrameStateInput(node_, 1);
FrameStateInfo state_info = OpParameter<FrameStateInfo>(frame_state); FrameStateInfo state_info = OpParameter<FrameStateInfo>(frame_state);
...@@ -516,8 +532,15 @@ Reduction JSTypedLowering::ReduceJSMultiply(Node* node) { ...@@ -516,8 +532,15 @@ Reduction JSTypedLowering::ReduceJSMultiply(Node* node) {
simplified()->SpeculativeNumberMultiply(feedback), Type::Number()); simplified()->SpeculativeNumberMultiply(feedback), Type::Number());
} }
r.ConvertInputsToNumber(); // If deoptimization is enabled we rely on type feedback.
return r.ChangeToPureOperator(simplified()->NumberMultiply(), Type::Number()); if (r.BothInputsAre(Type::PlainPrimitive()) ||
!(flags() & kDeoptimizationEnabled)) {
r.ConvertInputsToNumber();
return r.ChangeToPureOperator(simplified()->NumberMultiply(),
Type::Number());
}
return NoChange();
} }
Reduction JSTypedLowering::ReduceJSDivide(Node* node) { Reduction JSTypedLowering::ReduceJSDivide(Node* node) {
......
...@@ -35,6 +35,9 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) { ...@@ -35,6 +35,9 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) {
case IrOpcode::kJSStrictNotEqual: case IrOpcode::kJSStrictNotEqual:
return 0; return 0;
// Binary operations
case IrOpcode::kJSMultiply:
// Compare operations // Compare operations
case IrOpcode::kJSEqual: case IrOpcode::kJSEqual:
case IrOpcode::kJSNotEqual: case IrOpcode::kJSNotEqual:
...@@ -83,7 +86,6 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) { ...@@ -83,7 +86,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::kJSMultiply:
case IrOpcode::kJSAdd: case IrOpcode::kJSAdd:
case IrOpcode::kJSBitwiseAnd: case IrOpcode::kJSBitwiseAnd:
case IrOpcode::kJSBitwiseOr: case IrOpcode::kJSBitwiseOr:
......
...@@ -19,7 +19,9 @@ namespace compiler { ...@@ -19,7 +19,9 @@ namespace compiler {
class JSTypedLoweringTester : public HandleAndZoneScope { class JSTypedLoweringTester : public HandleAndZoneScope {
public: public:
explicit JSTypedLoweringTester(int num_parameters = 0) JSTypedLoweringTester(
int num_parameters = 0,
JSTypedLowering::Flags flags = JSTypedLowering::kDeoptimizationEnabled)
: isolate(main_isolate()), : isolate(main_isolate()),
binop(NULL), binop(NULL),
unop(NULL), unop(NULL),
...@@ -30,7 +32,8 @@ class JSTypedLoweringTester : public HandleAndZoneScope { ...@@ -30,7 +32,8 @@ class JSTypedLoweringTester : public HandleAndZoneScope {
deps(main_isolate(), main_zone()), deps(main_isolate(), main_zone()),
graph(main_zone()), graph(main_zone()),
typer(main_isolate(), &graph), typer(main_isolate(), &graph),
context_node(NULL) { context_node(NULL),
flags(flags) {
graph.SetStart(graph.NewNode(common.Start(num_parameters))); graph.SetStart(graph.NewNode(common.Start(num_parameters)));
graph.SetEnd(graph.NewNode(common.End(1), graph.start())); graph.SetEnd(graph.NewNode(common.End(1), graph.start()));
typer.Run(); typer.Run();
...@@ -47,6 +50,7 @@ class JSTypedLoweringTester : public HandleAndZoneScope { ...@@ -47,6 +50,7 @@ class JSTypedLoweringTester : public HandleAndZoneScope {
Graph graph; Graph graph;
Typer typer; Typer typer;
Node* context_node; Node* context_node;
JSTypedLowering::Flags flags;
BinaryOperationHints const binop_hints = BinaryOperationHints::Any(); BinaryOperationHints const binop_hints = BinaryOperationHints::Any();
CompareOperationHints const compare_hints = CompareOperationHints::Any(); CompareOperationHints const compare_hints = CompareOperationHints::Any();
...@@ -83,8 +87,7 @@ class JSTypedLoweringTester : public HandleAndZoneScope { ...@@ -83,8 +87,7 @@ class JSTypedLoweringTester : public HandleAndZoneScope {
&machine); &machine);
// TODO(titzer): mock the GraphReducer here for better unit testing. // TODO(titzer): mock the GraphReducer here for better unit testing.
GraphReducer graph_reducer(main_zone(), &graph); GraphReducer graph_reducer(main_zone(), &graph);
JSTypedLowering reducer(&graph_reducer, &deps, JSTypedLowering reducer(&graph_reducer, &deps, flags, &jsgraph,
JSTypedLowering::kDeoptimizationEnabled, &jsgraph,
main_zone()); main_zone());
Reduction reduction = reducer.Reduce(node); Reduction reduction = reducer.Reduce(node);
if (reduction.Changed()) return reduction.replacement(); if (reduction.Changed()) return reduction.replacement();
...@@ -749,8 +752,10 @@ TEST(RemoveToNumberEffects) { ...@@ -749,8 +752,10 @@ TEST(RemoveToNumberEffects) {
// Helper class for testing the reduction of a single binop. // Helper class for testing the reduction of a single binop.
class BinopEffectsTester { class BinopEffectsTester {
public: public:
explicit BinopEffectsTester(const Operator* op, Type* t0, Type* t1) BinopEffectsTester(
: R(), const Operator* op, Type* t0, Type* t1,
JSTypedLowering::Flags flags = JSTypedLowering::kDeoptimizationEnabled)
: R(0, flags),
p0(R.Parameter(t0, 0)), p0(R.Parameter(t0, 0)),
p1(R.Parameter(t1, 1)), p1(R.Parameter(t1, 1)),
binop(R.Binop(op, p0, p1)), binop(R.Binop(op, p0, p1)),
...@@ -930,7 +935,8 @@ TEST(OrderNumberBinopEffects1) { ...@@ -930,7 +935,8 @@ TEST(OrderNumberBinopEffects1) {
}; };
for (size_t j = 0; j < arraysize(ops); j += 2) { for (size_t j = 0; j < arraysize(ops); j += 2) {
BinopEffectsTester B(ops[j], Type::Symbol(), Type::Symbol()); BinopEffectsTester B(ops[j], Type::Symbol(), Type::Symbol(),
JSTypedLowering::kNoFlags);
CHECK_EQ(ops[j + 1]->opcode(), B.result->op()->opcode()); CHECK_EQ(ops[j + 1]->opcode(), B.result->op()->opcode());
Node* i0 = B.CheckConvertedInput(IrOpcode::kJSToNumber, 0, true); Node* i0 = B.CheckConvertedInput(IrOpcode::kJSToNumber, 0, true);
...@@ -956,7 +962,8 @@ TEST(OrderNumberBinopEffects2) { ...@@ -956,7 +962,8 @@ TEST(OrderNumberBinopEffects2) {
}; };
for (size_t j = 0; j < arraysize(ops); j += 2) { for (size_t j = 0; j < arraysize(ops); j += 2) {
BinopEffectsTester B(ops[j], Type::Number(), Type::Symbol()); BinopEffectsTester B(ops[j], Type::Number(), Type::Symbol(),
JSTypedLowering::kNoFlags);
Node* i0 = B.CheckNoOp(0); Node* i0 = B.CheckNoOp(0);
Node* i1 = B.CheckConvertedInput(IrOpcode::kJSToNumber, 1, true); Node* i1 = B.CheckConvertedInput(IrOpcode::kJSToNumber, 1, true);
...@@ -969,7 +976,8 @@ TEST(OrderNumberBinopEffects2) { ...@@ -969,7 +976,8 @@ TEST(OrderNumberBinopEffects2) {
} }
for (size_t j = 0; j < arraysize(ops); j += 2) { for (size_t j = 0; j < arraysize(ops); j += 2) {
BinopEffectsTester B(ops[j], Type::Symbol(), Type::Number()); BinopEffectsTester B(ops[j], Type::Symbol(), Type::Number(),
JSTypedLowering::kNoFlags);
Node* i0 = B.CheckConvertedInput(IrOpcode::kJSToNumber, 0, true); Node* i0 = B.CheckConvertedInput(IrOpcode::kJSToNumber, 0, true);
Node* i1 = B.CheckNoOp(1); Node* i1 = B.CheckNoOp(1);
......
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