From: bmeurer@chromium.org Date: Mon, 18 Aug 2014 11:36:06 +0000 (+0000) Subject: [turbofan] Add new ControlEffect and Finish operators. X-Git-Tag: upstream/4.7.83~7589 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d9fe1e71c3e04d864746e7e8dd3990e14538635b;p=platform%2Fupstream%2Fv8.git [turbofan] Add new ControlEffect and Finish operators. Fix the ChangeLowering to properly use ControlEffect nodes to turn the control output of IfTrue nodes into an effect input for the Load nodes, and to properly use Finish nodes to ensure that allocation and store were both performed prior to actually using the allocated heap number. TEST=compiler-unittests R=jarin@chromium.org Review URL: https://codereview.chromium.org/479163002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@23149 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- diff --git a/src/compiler/change-lowering.cc b/src/compiler/change-lowering.cc index 86c473f..4863f8f 100644 --- a/src/compiler/change-lowering.cc +++ b/src/compiler/change-lowering.cc @@ -94,7 +94,6 @@ Reduction ChangeLowering::ChangeInt32ToTagged(Node* val, Node* effect, Node* if_true = graph()->NewNode(common()->IfTrue(), branch); Node* number = graph()->NewNode(machine()->ChangeInt32ToFloat64(), val); - // TODO(bmeurer): Inline allocation if possible. const Runtime::Function* fn = Runtime::FunctionForId(Runtime::kAllocateHeapNumber); DCHECK_EQ(0, fn->nargs); @@ -103,17 +102,17 @@ Reduction ChangeLowering::ChangeInt32ToTagged(Node* val, Node* effect, Node* heap_number = graph()->NewNode( common()->Call(desc), jsgraph()->CEntryStubConstant(), jsgraph()->ExternalConstant(ExternalReference(fn, isolate())), - jsgraph()->ZeroConstant(), context, effect, if_true); - + jsgraph()->Int32Constant(fn->nargs), context, effect, if_true); Node* store = graph()->NewNode( machine()->Store(kMachFloat64, kNoWriteBarrier), heap_number, - HeapNumberValueIndexConstant(), number, effect, heap_number); + HeapNumberValueIndexConstant(), number, heap_number, if_true); + Node* finish = graph()->NewNode(common()->Finish(1), heap_number, store); Node* if_false = graph()->NewNode(common()->IfFalse(), branch); Node* smi = graph()->NewNode(common()->Projection(0), add); - Node* merge = graph()->NewNode(common()->Merge(2), store, if_false); - Node* phi = graph()->NewNode(common()->Phi(2), heap_number, smi, merge); + Node* merge = graph()->NewNode(common()->Merge(2), if_true, if_false); + Node* phi = graph()->NewNode(common()->Phi(2), finish, smi, merge); return Replace(phi); } @@ -128,8 +127,9 @@ Reduction ChangeLowering::ChangeTaggedToFloat64(Node* val, Node* effect, Node* branch = graph()->NewNode(common()->Branch(), tag, control); Node* if_true = graph()->NewNode(common()->IfTrue(), branch); - Node* load = graph()->NewNode(machine()->Load(kMachFloat64), val, - HeapNumberValueIndexConstant(), if_true); + Node* load = graph()->NewNode( + machine()->Load(kMachFloat64), val, HeapNumberValueIndexConstant(), + graph()->NewNode(common()->ControlEffect(), if_true)); Node* if_false = graph()->NewNode(common()->IfFalse(), branch); Node* integer = diff --git a/src/compiler/common-operator.h b/src/compiler/common-operator.h index 3b581ae..2c5f1b6 100644 --- a/src/compiler/common-operator.h +++ b/src/compiler/common-operator.h @@ -130,6 +130,15 @@ class CommonOperatorBuilder { return new (zone_) Operator1(IrOpcode::kEffectPhi, Operator::kPure, 0, 0, "EffectPhi", arguments); } + Operator* ControlEffect() { + return new (zone_) SimpleOperator(IrOpcode::kControlEffect, Operator::kPure, + 0, 0, "ControlEffect"); + } + Operator* Finish(int arguments) { + DCHECK(arguments > 0); // Disallow empty finishes. + return new (zone_) Operator1(IrOpcode::kFinish, Operator::kPure, 1, 1, + "Finish", arguments); + } Operator* StateValues(int arguments) { return new (zone_) Operator1(IrOpcode::kStateValues, Operator::kPure, arguments, 1, "StateValues", arguments); diff --git a/src/compiler/opcodes.h b/src/compiler/opcodes.h index df15059..a3e1ba4 100644 --- a/src/compiler/opcodes.h +++ b/src/compiler/opcodes.h @@ -33,6 +33,8 @@ #define INNER_OP_LIST(V) \ V(Phi) \ V(EffectPhi) \ + V(ControlEffect) \ + V(Finish) \ V(FrameState) \ V(StateValues) \ V(Call) \ diff --git a/src/compiler/operator-properties-inl.h b/src/compiler/operator-properties-inl.h index 42833fd..7830c1a 100644 --- a/src/compiler/operator-properties-inl.h +++ b/src/compiler/operator-properties-inl.h @@ -42,7 +42,8 @@ inline int OperatorProperties::GetContextInputCount(Operator* op) { } inline int OperatorProperties::GetEffectInputCount(Operator* op) { - if (op->opcode() == IrOpcode::kEffectPhi) { + if (op->opcode() == IrOpcode::kEffectPhi || + op->opcode() == IrOpcode::kFinish) { return static_cast*>(op)->parameter(); } if (op->HasProperty(Operator::kNoRead) && op->HasProperty(Operator::kNoWrite)) @@ -54,6 +55,7 @@ inline int OperatorProperties::GetControlInputCount(Operator* op) { switch (op->opcode()) { case IrOpcode::kPhi: case IrOpcode::kEffectPhi: + case IrOpcode::kControlEffect: return 1; #define OPCODE_CASE(x) case IrOpcode::k##x: CONTROL_OP_LIST(OPCODE_CASE) @@ -87,7 +89,9 @@ inline bool OperatorProperties::HasValueOutput(Operator* op) { } inline bool OperatorProperties::HasEffectOutput(Operator* op) { - return op->opcode() == IrOpcode::kStart || GetEffectInputCount(op) > 0; + return op->opcode() == IrOpcode::kStart || + op->opcode() == IrOpcode::kControlEffect || + (op->opcode() != IrOpcode::kFinish && GetEffectInputCount(op) > 0); } inline bool OperatorProperties::HasControlOutput(Operator* op) { diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc index 2aa1869..6a0127e 100644 --- a/src/compiler/typer.cc +++ b/src/compiler/typer.cc @@ -267,6 +267,14 @@ Bounds Typer::Visitor::TypeEffectPhi(Node* node) { } +Bounds Typer::Visitor::TypeControlEffect(Node* node) { + return Bounds(Type::None(zone())); +} + + +Bounds Typer::Visitor::TypeFinish(Node* node) { return OperandType(node, 0); } + + Bounds Typer::Visitor::TypeFrameState(Node* node) { return Bounds(Type::None(zone())); } diff --git a/test/compiler-unittests/change-lowering-unittest.cc b/test/compiler-unittests/change-lowering-unittest.cc index 45e001b..329acff 100644 --- a/test/compiler-unittests/change-lowering-unittest.cc +++ b/test/compiler-unittests/change-lowering-unittest.cc @@ -117,34 +117,33 @@ TARGET_TEST_F(ChangeLowering32Test, ChangeInt32ToTagged) { ASSERT_TRUE(reduction.Changed()); Node* phi = reduction.replacement(); - ASSERT_EQ(IrOpcode::kPhi, phi->opcode()); - - Node* smi = NodeProperties::GetValueInput(phi, 1); - ASSERT_THAT(smi, IsProjection(0, IsInt32AddWithOverflow(val, val))); - - Node* heap_number = NodeProperties::GetValueInput(phi, 0); - ASSERT_EQ(IrOpcode::kCall, heap_number->opcode()); - - Node* merge = NodeProperties::GetControlInput(phi); - ASSERT_EQ(IrOpcode::kMerge, merge->opcode()); - + Capture add, branch, heap_number, if_true; const int32_t kValueOffset = kHeapNumberValueOffset - kHeapObjectTag; - EXPECT_THAT(NodeProperties::GetControlInput(merge, 0), - IsStore(kMachFloat64, kNoWriteBarrier, heap_number, + EXPECT_THAT( + phi, + IsPhi( + IsFinish( + AllOf( + CaptureEq(&heap_number), + IsCall( + _, IsHeapConstant( + PrintableUnique::CreateImmovable( + zone(), CEntryStub(isolate(), 1).GetCode())), + IsExternalConstant(ExternalReference( + Runtime::FunctionForId(Runtime::kAllocateHeapNumber), + isolate())), + IsInt32Constant(0), IsNumberConstant(0.0), + graph()->start(), CaptureEq(&if_true))), + IsStore(kMachFloat64, kNoWriteBarrier, CaptureEq(&heap_number), IsInt32Constant(kValueOffset), - IsChangeInt32ToFloat64(val), _, heap_number)); - - Node* if_true = NodeProperties::GetControlInput(heap_number); - ASSERT_EQ(IrOpcode::kIfTrue, if_true->opcode()); - - Node* if_false = NodeProperties::GetControlInput(merge, 1); - ASSERT_EQ(IrOpcode::kIfFalse, if_false->opcode()); - - Node* branch = NodeProperties::GetControlInput(if_true); - EXPECT_EQ(branch, NodeProperties::GetControlInput(if_false)); - EXPECT_THAT(branch, - IsBranch(IsProjection(1, IsInt32AddWithOverflow(val, val)), - graph()->start())); + IsChangeInt32ToFloat64(val), CaptureEq(&heap_number), + CaptureEq(&if_true))), + IsProjection( + 0, AllOf(CaptureEq(&add), IsInt32AddWithOverflow(val, val))), + IsMerge(AllOf(CaptureEq(&if_true), IsIfTrue(CaptureEq(&branch))), + IsIfFalse(AllOf(CaptureEq(&branch), + IsBranch(IsProjection(1, CaptureEq(&add)), + graph()->start())))))); } @@ -161,17 +160,21 @@ TARGET_TEST_F(ChangeLowering32Test, ChangeTaggedToFloat64) { kSmiTagSize + SmiTagging::kSmiShiftSize; const int32_t kValueOffset = kHeapNumberValueOffset - kHeapObjectTag; Node* phi = reduction.replacement(); - Capture branch; + Capture branch, if_true; EXPECT_THAT( phi, - IsPhi(IsLoad(kMachFloat64, val, IsInt32Constant(kValueOffset), _), - IsChangeInt32ToFloat64( - IsWord32Sar(val, IsInt32Constant(kShiftAmount))), - IsMerge(IsIfTrue(AllOf( + IsPhi( + IsLoad(kMachFloat64, val, IsInt32Constant(kValueOffset), + IsControlEffect(CaptureEq(&if_true))), + IsChangeInt32ToFloat64( + IsWord32Sar(val, IsInt32Constant(kShiftAmount))), + IsMerge( + AllOf(CaptureEq(&if_true), + IsIfTrue(AllOf( CaptureEq(&branch), IsBranch(IsWord32And(val, IsInt32Constant(kSmiTagMask)), - graph()->start()))), - IsIfFalse(CaptureEq(&branch))))); + graph()->start())))), + IsIfFalse(CaptureEq(&branch))))); } @@ -218,17 +221,21 @@ TARGET_TEST_F(ChangeLowering64Test, ChangeTaggedToFloat64) { kSmiTagSize + SmiTagging::kSmiShiftSize; const int32_t kValueOffset = kHeapNumberValueOffset - kHeapObjectTag; Node* phi = reduction.replacement(); - Capture branch; + Capture branch, if_true; EXPECT_THAT( phi, - IsPhi(IsLoad(kMachFloat64, val, IsInt32Constant(kValueOffset), _), - IsChangeInt32ToFloat64(IsConvertInt64ToInt32( - IsWord64Sar(val, IsInt32Constant(kShiftAmount)))), - IsMerge(IsIfTrue(AllOf( + IsPhi( + IsLoad(kMachFloat64, val, IsInt32Constant(kValueOffset), + IsControlEffect(CaptureEq(&if_true))), + IsChangeInt32ToFloat64(IsConvertInt64ToInt32( + IsWord64Sar(val, IsInt32Constant(kShiftAmount)))), + IsMerge( + AllOf(CaptureEq(&if_true), + IsIfTrue(AllOf( CaptureEq(&branch), IsBranch(IsWord64And(val, IsInt32Constant(kSmiTagMask)), - graph()->start()))), - IsIfFalse(CaptureEq(&branch))))); + graph()->start())))), + IsIfFalse(CaptureEq(&branch))))); } } // namespace compiler diff --git a/test/compiler-unittests/common-operator-unittest.cc b/test/compiler-unittests/common-operator-unittest.cc index fa5af1e..f034127 100644 --- a/test/compiler-unittests/common-operator-unittest.cc +++ b/test/compiler-unittests/common-operator-unittest.cc @@ -15,6 +15,30 @@ CommonOperatorTest::CommonOperatorTest() : common_(zone()) {} CommonOperatorTest::~CommonOperatorTest() {} + +TEST_F(CommonOperatorTest, ControlEffect) { + Operator* op = common()->ControlEffect(); + EXPECT_EQ(1, OperatorProperties::GetControlInputCount(op)); + EXPECT_EQ(1, OperatorProperties::GetTotalInputCount(op)); + EXPECT_EQ(0, OperatorProperties::GetControlOutputCount(op)); + EXPECT_EQ(1, OperatorProperties::GetEffectOutputCount(op)); + EXPECT_EQ(0, OperatorProperties::GetValueOutputCount(op)); +} + + +TEST_F(CommonOperatorTest, Finish) { + static const int kArguments[] = {1, 5, 6, 42, 100, 10000, kMaxInt}; + TRACED_FOREACH(int, arguments, kArguments) { + Operator* op = common()->Finish(arguments); + EXPECT_EQ(1, OperatorProperties::GetValueInputCount(op)); + EXPECT_EQ(arguments, OperatorProperties::GetEffectInputCount(op)); + EXPECT_EQ(arguments + 1, OperatorProperties::GetTotalInputCount(op)); + EXPECT_EQ(0, OperatorProperties::GetControlOutputCount(op)); + EXPECT_EQ(0, OperatorProperties::GetEffectOutputCount(op)); + EXPECT_EQ(1, OperatorProperties::GetValueOutputCount(op)); + } +} + } // namespace compiler } // namespace internal } // namespace v8 diff --git a/test/compiler-unittests/graph-unittest.cc b/test/compiler-unittests/graph-unittest.cc index 6c946de..e5b413c 100644 --- a/test/compiler-unittests/graph-unittest.cc +++ b/test/compiler-unittests/graph-unittest.cc @@ -22,6 +22,12 @@ inline std::ostream& operator<<(std::ostream& os, const PrintableUnique& value) { return os << value.string(); } +inline std::ostream& operator<<(std::ostream& os, + const ExternalReference& value) { + OStringStream ost; + compiler::StaticParameterTraits::PrintTo(ost, value); + return os << ost.c_str(); +} namespace compiler { @@ -66,7 +72,8 @@ class NodeMatcher : public MatcherInterface { return false; } if (node->opcode() != opcode_) { - *listener << "whose opcode is " << IrOpcode::Mnemonic(node->opcode()); + *listener << "whose opcode is " << IrOpcode::Mnemonic(node->opcode()) + << " but should have been " << IrOpcode::Mnemonic(opcode_); return false; } return true; @@ -141,10 +148,11 @@ class IsMergeMatcher V8_FINAL : public NodeMatcher { }; -class IsIfTrueMatcher V8_FINAL : public NodeMatcher { +class IsControl1Matcher V8_FINAL : public NodeMatcher { public: - explicit IsIfTrueMatcher(const Matcher& control_matcher) - : NodeMatcher(IrOpcode::kIfTrue), control_matcher_(control_matcher) {} + IsControl1Matcher(IrOpcode::Value opcode, + const Matcher& control_matcher) + : NodeMatcher(opcode), control_matcher_(control_matcher) {} virtual void DescribeTo(std::ostream* os) const V8_OVERRIDE { NodeMatcher::DescribeTo(os); @@ -165,27 +173,35 @@ class IsIfTrueMatcher V8_FINAL : public NodeMatcher { }; -class IsIfFalseMatcher V8_FINAL : public NodeMatcher { +class IsFinishMatcher V8_FINAL : public NodeMatcher { public: - explicit IsIfFalseMatcher(const Matcher& control_matcher) - : NodeMatcher(IrOpcode::kIfFalse), control_matcher_(control_matcher) {} + IsFinishMatcher(const Matcher& value_matcher, + const Matcher& effect_matcher) + : NodeMatcher(IrOpcode::kFinish), + value_matcher_(value_matcher), + effect_matcher_(effect_matcher) {} virtual void DescribeTo(std::ostream* os) const V8_OVERRIDE { NodeMatcher::DescribeTo(os); - *os << " whose control ("; - control_matcher_.DescribeTo(os); + *os << " whose value ("; + value_matcher_.DescribeTo(os); + *os << ") and effect ("; + effect_matcher_.DescribeTo(os); *os << ")"; } virtual bool MatchAndExplain(Node* node, MatchResultListener* listener) const V8_OVERRIDE { return (NodeMatcher::MatchAndExplain(node, listener) && - PrintMatchAndExplain(NodeProperties::GetControlInput(node), - "control", control_matcher_, listener)); + PrintMatchAndExplain(NodeProperties::GetValueInput(node, 0), + "value", value_matcher_, listener) && + PrintMatchAndExplain(NodeProperties::GetEffectInput(node), "effect", + effect_matcher_, listener)); } private: - const Matcher control_matcher_; + const Matcher value_matcher_; + const Matcher effect_matcher_; }; @@ -285,6 +301,71 @@ class IsProjectionMatcher V8_FINAL : public NodeMatcher { }; +class IsCallMatcher V8_FINAL : public NodeMatcher { + public: + IsCallMatcher(const Matcher& descriptor_matcher, + const Matcher& value0_matcher, + const Matcher& value1_matcher, + const Matcher& value2_matcher, + const Matcher& value3_matcher, + const Matcher& effect_matcher, + const Matcher& control_matcher) + : NodeMatcher(IrOpcode::kCall), + descriptor_matcher_(descriptor_matcher), + value0_matcher_(value0_matcher), + value1_matcher_(value1_matcher), + value2_matcher_(value2_matcher), + value3_matcher_(value3_matcher), + effect_matcher_(effect_matcher), + control_matcher_(control_matcher) {} + + virtual void DescribeTo(std::ostream* os) const V8_OVERRIDE { + NodeMatcher::DescribeTo(os); + *os << " whose value0 ("; + value0_matcher_.DescribeTo(os); + *os << ") and value1 ("; + value1_matcher_.DescribeTo(os); + *os << ") and value2 ("; + value2_matcher_.DescribeTo(os); + *os << ") and value3 ("; + value3_matcher_.DescribeTo(os); + *os << ") and effect ("; + effect_matcher_.DescribeTo(os); + *os << ") and control ("; + control_matcher_.DescribeTo(os); + *os << ")"; + } + + virtual bool MatchAndExplain(Node* node, MatchResultListener* listener) const + V8_OVERRIDE { + return (NodeMatcher::MatchAndExplain(node, listener) && + PrintMatchAndExplain(OpParameter(node), + "descriptor", descriptor_matcher_, listener) && + PrintMatchAndExplain(NodeProperties::GetValueInput(node, 0), + "value0", value0_matcher_, listener) && + PrintMatchAndExplain(NodeProperties::GetValueInput(node, 1), + "value1", value1_matcher_, listener) && + PrintMatchAndExplain(NodeProperties::GetValueInput(node, 2), + "value2", value2_matcher_, listener) && + PrintMatchAndExplain(NodeProperties::GetValueInput(node, 3), + "value3", value3_matcher_, listener) && + PrintMatchAndExplain(NodeProperties::GetEffectInput(node), "effect", + effect_matcher_, listener) && + PrintMatchAndExplain(NodeProperties::GetControlInput(node), + "control", control_matcher_, listener)); + } + + private: + const Matcher descriptor_matcher_; + const Matcher value0_matcher_; + const Matcher value1_matcher_; + const Matcher value2_matcher_; + const Matcher value3_matcher_; + const Matcher effect_matcher_; + const Matcher control_matcher_; +}; + + class IsLoadMatcher V8_FINAL : public NodeMatcher { public: IsLoadMatcher(const Matcher& type_matcher, @@ -470,18 +551,32 @@ Matcher IsMerge(const Matcher& control0_matcher, Matcher IsIfTrue(const Matcher& control_matcher) { - return MakeMatcher(new IsIfTrueMatcher(control_matcher)); + return MakeMatcher(new IsControl1Matcher(IrOpcode::kIfTrue, control_matcher)); } Matcher IsIfFalse(const Matcher& control_matcher) { - return MakeMatcher(new IsIfFalseMatcher(control_matcher)); + return MakeMatcher( + new IsControl1Matcher(IrOpcode::kIfFalse, control_matcher)); } -Matcher IsInt32Constant(const Matcher& value_matcher) { +Matcher IsControlEffect(const Matcher& control_matcher) { return MakeMatcher( - new IsConstantMatcher(IrOpcode::kInt32Constant, value_matcher)); + new IsControl1Matcher(IrOpcode::kControlEffect, control_matcher)); +} + + +Matcher IsFinish(const Matcher& value_matcher, + const Matcher& effect_matcher) { + return MakeMatcher(new IsFinishMatcher(value_matcher, effect_matcher)); +} + + +Matcher IsExternalConstant( + const Matcher& value_matcher) { + return MakeMatcher(new IsConstantMatcher( + IrOpcode::kExternalConstant, value_matcher)); } @@ -492,6 +587,18 @@ Matcher IsHeapConstant( } +Matcher IsInt32Constant(const Matcher& value_matcher) { + return MakeMatcher( + new IsConstantMatcher(IrOpcode::kInt32Constant, value_matcher)); +} + + +Matcher IsNumberConstant(const Matcher& value_matcher) { + return MakeMatcher( + new IsConstantMatcher(IrOpcode::kNumberConstant, value_matcher)); +} + + Matcher IsPhi(const Matcher& value0_matcher, const Matcher& value1_matcher, const Matcher& merge_matcher) { @@ -506,6 +613,19 @@ Matcher IsProjection(const Matcher& index_matcher, } +Matcher IsCall(const Matcher& descriptor_matcher, + const Matcher& value0_matcher, + const Matcher& value1_matcher, + const Matcher& value2_matcher, + const Matcher& value3_matcher, + const Matcher& effect_matcher, + const Matcher& control_matcher) { + return MakeMatcher(new IsCallMatcher( + descriptor_matcher, value0_matcher, value1_matcher, value2_matcher, + value3_matcher, effect_matcher, control_matcher)); +} + + Matcher IsLoad(const Matcher& type_matcher, const Matcher& base_matcher, const Matcher& index_matcher, diff --git a/test/compiler-unittests/graph-unittest.h b/test/compiler-unittests/graph-unittest.h index a411738..3740919 100644 --- a/test/compiler-unittests/graph-unittest.h +++ b/test/compiler-unittests/graph-unittest.h @@ -41,14 +41,27 @@ Matcher IsMerge(const Matcher& control0_matcher, const Matcher& control1_matcher); Matcher IsIfTrue(const Matcher& control_matcher); Matcher IsIfFalse(const Matcher& control_matcher); +Matcher IsControlEffect(const Matcher& control_matcher); +Matcher IsFinish(const Matcher& value_matcher, + const Matcher& effect_matcher); +Matcher IsExternalConstant( + const Matcher& value_matcher); Matcher IsHeapConstant( const Matcher >& value_matcher); Matcher IsInt32Constant(const Matcher& value_matcher); +Matcher IsNumberConstant(const Matcher& value_matcher); Matcher IsPhi(const Matcher& value0_matcher, const Matcher& value1_matcher, const Matcher& merge_matcher); Matcher IsProjection(const Matcher& index_matcher, const Matcher& base_matcher); +Matcher IsCall(const Matcher& descriptor_matcher, + const Matcher& value0_matcher, + const Matcher& value1_matcher, + const Matcher& value2_matcher, + const Matcher& value3_matcher, + const Matcher& effect_matcher, + const Matcher& control_matcher); Matcher IsLoad(const Matcher& type_matcher, const Matcher& base_matcher,