From 277fac44ffe9e5824964c4a252138ff25c2d8e82 Mon Sep 17 00:00:00 2001 From: mstarzinger Date: Tue, 5 Jul 2016 02:03:52 -0700 Subject: [PATCH] [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} --- src/compiler/js-typed-lowering.cc | 33 ++++++++++++++++--- src/compiler/operator-properties.cc | 4 ++- .../cctest/compiler/test-js-typed-lowering.cc | 26 ++++++++++----- 3 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc index 7e2d10200d..809eb49412 100644 --- a/src/compiler/js-typed-lowering.cc +++ b/src/compiler/js-typed-lowering.cc @@ -34,7 +34,7 @@ class JSBinopReduction final { } DCHECK_NE(0, node_->op()->ControlOutputCount()); 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::Hint combined = hints.combined(); if (combined == BinaryOperationHints::kSignedSmall || @@ -155,7 +155,7 @@ class JSBinopReduction final { DCHECK_EQ(1, node_->op()->EffectOutputCount()); DCHECK_EQ(1, node_->op()->ControlInputCount()); 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()); // Reconnect the control output to bypass the IfSuccess node and @@ -175,7 +175,9 @@ class JSBinopReduction final { } // 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::FirstContextIndex(node_)); @@ -230,6 +232,13 @@ class JSBinopReduction final { Node* node_; // The original node. 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(dummy_state).bailout_id().IsNone()); + return dummy_state; + } + Node* frame_state = NodeProperties::GetFrameStateInput(node_, 1); FrameStateInfo state_info = OpParameter(frame_state); @@ -261,6 +270,13 @@ class JSBinopReduction final { } 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(dummy_state).bailout_id().IsNone()); + return dummy_state; + } + Node* frame_state = NodeProperties::GetFrameStateInput(node_, 1); FrameStateInfo state_info = OpParameter(frame_state); @@ -516,8 +532,15 @@ Reduction JSTypedLowering::ReduceJSMultiply(Node* node) { simplified()->SpeculativeNumberMultiply(feedback), Type::Number()); } - r.ConvertInputsToNumber(); - return r.ChangeToPureOperator(simplified()->NumberMultiply(), Type::Number()); + // If deoptimization is enabled we rely on type feedback. + if (r.BothInputsAre(Type::PlainPrimitive()) || + !(flags() & kDeoptimizationEnabled)) { + r.ConvertInputsToNumber(); + return r.ChangeToPureOperator(simplified()->NumberMultiply(), + Type::Number()); + } + + return NoChange(); } Reduction JSTypedLowering::ReduceJSDivide(Node* node) { diff --git a/src/compiler/operator-properties.cc b/src/compiler/operator-properties.cc index 43b00761fe..2fdd088cd3 100644 --- a/src/compiler/operator-properties.cc +++ b/src/compiler/operator-properties.cc @@ -35,6 +35,9 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) { case IrOpcode::kJSStrictNotEqual: return 0; + // Binary operations + case IrOpcode::kJSMultiply: + // Compare operations case IrOpcode::kJSEqual: case IrOpcode::kJSNotEqual: @@ -83,7 +86,6 @@ int OperatorProperties::GetFrameStateInputCount(const Operator* op) { // 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 // state so that we can resume before the operation. - case IrOpcode::kJSMultiply: case IrOpcode::kJSAdd: case IrOpcode::kJSBitwiseAnd: case IrOpcode::kJSBitwiseOr: diff --git a/test/cctest/compiler/test-js-typed-lowering.cc b/test/cctest/compiler/test-js-typed-lowering.cc index f3380dd271..17886673c6 100644 --- a/test/cctest/compiler/test-js-typed-lowering.cc +++ b/test/cctest/compiler/test-js-typed-lowering.cc @@ -19,7 +19,9 @@ namespace compiler { class JSTypedLoweringTester : public HandleAndZoneScope { public: - explicit JSTypedLoweringTester(int num_parameters = 0) + JSTypedLoweringTester( + int num_parameters = 0, + JSTypedLowering::Flags flags = JSTypedLowering::kDeoptimizationEnabled) : isolate(main_isolate()), binop(NULL), unop(NULL), @@ -30,7 +32,8 @@ class JSTypedLoweringTester : public HandleAndZoneScope { deps(main_isolate(), main_zone()), graph(main_zone()), typer(main_isolate(), &graph), - context_node(NULL) { + context_node(NULL), + flags(flags) { graph.SetStart(graph.NewNode(common.Start(num_parameters))); graph.SetEnd(graph.NewNode(common.End(1), graph.start())); typer.Run(); @@ -47,6 +50,7 @@ class JSTypedLoweringTester : public HandleAndZoneScope { Graph graph; Typer typer; Node* context_node; + JSTypedLowering::Flags flags; BinaryOperationHints const binop_hints = BinaryOperationHints::Any(); CompareOperationHints const compare_hints = CompareOperationHints::Any(); @@ -83,8 +87,7 @@ class JSTypedLoweringTester : public HandleAndZoneScope { &machine); // TODO(titzer): mock the GraphReducer here for better unit testing. GraphReducer graph_reducer(main_zone(), &graph); - JSTypedLowering reducer(&graph_reducer, &deps, - JSTypedLowering::kDeoptimizationEnabled, &jsgraph, + JSTypedLowering reducer(&graph_reducer, &deps, flags, &jsgraph, main_zone()); Reduction reduction = reducer.Reduce(node); if (reduction.Changed()) return reduction.replacement(); @@ -749,8 +752,10 @@ TEST(RemoveToNumberEffects) { // Helper class for testing the reduction of a single binop. class BinopEffectsTester { public: - explicit BinopEffectsTester(const Operator* op, Type* t0, Type* t1) - : R(), + BinopEffectsTester( + const Operator* op, Type* t0, Type* t1, + JSTypedLowering::Flags flags = JSTypedLowering::kDeoptimizationEnabled) + : R(0, flags), p0(R.Parameter(t0, 0)), p1(R.Parameter(t1, 1)), binop(R.Binop(op, p0, p1)), @@ -930,7 +935,8 @@ TEST(OrderNumberBinopEffects1) { }; 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()); Node* i0 = B.CheckConvertedInput(IrOpcode::kJSToNumber, 0, true); @@ -956,7 +962,8 @@ TEST(OrderNumberBinopEffects2) { }; 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* i1 = B.CheckConvertedInput(IrOpcode::kJSToNumber, 1, true); @@ -969,7 +976,8 @@ TEST(OrderNumberBinopEffects2) { } 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* i1 = B.CheckNoOp(1);