From b9afcdcefb7f0e55f58b5f538df5ccac39480a80 Mon Sep 17 00:00:00 2001 From: "bmeurer@chromium.org" Date: Wed, 1 Oct 2014 11:08:37 +0000 Subject: [PATCH] [turbofan] Add control input to Load and LoadElements. Also remove the now obsolete ControlEffect operator. TEST=cctest,mjsunit,unittests R=mstarzinger@chromium.org Review URL: https://codereview.chromium.org/620803003 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24359 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/compiler/ast-graph-builder.cc | 1 - src/compiler/change-lowering.cc | 4 +-- src/compiler/common-operator.cc | 12 --------- src/compiler/common-operator.h | 1 - src/compiler/js-generic-lowering.cc | 9 +++---- src/compiler/js-typed-lowering.cc | 8 +++--- src/compiler/opcodes.h | 1 - src/compiler/operator-properties-inl.h | 4 +-- src/compiler/simplified-lowering.cc | 1 + src/compiler/typer.cc | 6 ----- test/cctest/compiler/test-changes-lowering.cc | 6 ++--- test/cctest/compiler/test-simplified-lowering.cc | 6 ++--- .../unittests/compiler/change-lowering-unittest.cc | 30 +++++++++------------- .../unittests/compiler/common-operator-unittest.cc | 3 +-- test/unittests/compiler/graph-unittest.cc | 26 ++++++++++--------- test/unittests/compiler/graph-unittest.h | 4 +-- .../compiler/machine-operator-unittest.cc | 4 +-- .../compiler/simplified-operator-unittest.cc | 4 +-- 18 files changed, 51 insertions(+), 79 deletions(-) diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index d33b05a..3ac4b46 100644 --- a/src/compiler/ast-graph-builder.cc +++ b/src/compiler/ast-graph-builder.cc @@ -1975,7 +1975,6 @@ Node* AstGraphBuilder::BuildVariableAssignment(Variable* variable, Node* value, Node* AstGraphBuilder::BuildLoadObjectField(Node* object, int offset) { - // TODO(sigurds) Use simplified load here once it is ready. Node* field_load = NewNode(jsgraph()->machine()->Load(kMachAnyTagged), object, jsgraph()->Int32Constant(offset - kHeapObjectTag)); return field_load; diff --git a/src/compiler/change-lowering.cc b/src/compiler/change-lowering.cc index 5b66d55..d1cbdb8 100644 --- a/src/compiler/change-lowering.cc +++ b/src/compiler/change-lowering.cc @@ -97,8 +97,8 @@ Node* ChangeLowering::ChangeSmiToInt32(Node* value) { Node* ChangeLowering::LoadHeapNumberValue(Node* value, Node* control) { return graph()->NewNode(machine()->Load(kMachFloat64), value, - HeapNumberValueIndexConstant(), - graph()->NewNode(common()->ControlEffect(), control)); + HeapNumberValueIndexConstant(), graph()->start(), + control); } diff --git a/src/compiler/common-operator.cc b/src/compiler/common-operator.cc index 4ce98aa..f0f158b 100644 --- a/src/compiler/common-operator.cc +++ b/src/compiler/common-operator.cc @@ -75,13 +75,6 @@ struct CommonOperatorBuilderImpl FINAL { Name##Operator k##Name##Operator; SHARED_OP_LIST(SHARED) #undef SHARED - - struct ControlEffectOperator FINAL : public SimpleOperator { - ControlEffectOperator() - : SimpleOperator(IrOpcode::kControlEffect, Operator::kPure, 0, 0, - "ControlEffect") {} - }; - ControlEffectOperator kControlEffectOperator; }; @@ -189,11 +182,6 @@ const Operator* CommonOperatorBuilder::EffectPhi(int arguments) { } -const Operator* CommonOperatorBuilder::ControlEffect() { - return &impl_.kControlEffectOperator; -} - - const Operator* CommonOperatorBuilder::ValueEffect(int arguments) { DCHECK(arguments > 0); // Disallow empty value effects. return new (zone()) SimpleOperator(IrOpcode::kValueEffect, Operator::kPure, diff --git a/src/compiler/common-operator.h b/src/compiler/common-operator.h index 2e30a04..941fb17 100644 --- a/src/compiler/common-operator.h +++ b/src/compiler/common-operator.h @@ -111,7 +111,6 @@ class CommonOperatorBuilder FINAL { const Operator* Phi(MachineType type, int arguments); const Operator* EffectPhi(int arguments); - const Operator* ControlEffect(); const Operator* ValueEffect(int arguments); const Operator* Finish(int arguments); const Operator* StateValues(int arguments); diff --git a/src/compiler/js-generic-lowering.cc b/src/compiler/js-generic-lowering.cc index 3708c7f..5affb77 100644 --- a/src/compiler/js-generic-lowering.cc +++ b/src/compiler/js-generic-lowering.cc @@ -328,33 +328,30 @@ void JSGenericLowering::LowerJSInstanceOf(Node* node) { void JSGenericLowering::LowerJSLoadContext(Node* node) { const ContextAccess& access = ContextAccessOf(node->op()); - // TODO(mstarzinger): Use simplified operators instead of machine operators - // here so that load/store optimization can be applied afterwards. for (size_t i = 0; i < access.depth(); ++i) { node->ReplaceInput( 0, graph()->NewNode( machine()->Load(kMachAnyTagged), NodeProperties::GetValueInput(node, 0), Int32Constant(Context::SlotOffset(Context::PREVIOUS_INDEX)), - NodeProperties::GetEffectInput(node))); + NodeProperties::GetEffectInput(node), graph()->start())); } node->ReplaceInput( 1, Int32Constant(Context::SlotOffset(static_cast(access.index())))); + node->AppendInput(zone(), graph()->start()); PatchOperator(node, machine()->Load(kMachAnyTagged)); } void JSGenericLowering::LowerJSStoreContext(Node* node) { const ContextAccess& access = ContextAccessOf(node->op()); - // TODO(mstarzinger): Use simplified operators instead of machine operators - // here so that load/store optimization can be applied afterwards. for (size_t i = 0; i < access.depth(); ++i) { node->ReplaceInput( 0, graph()->NewNode( machine()->Load(kMachAnyTagged), NodeProperties::GetValueInput(node, 0), Int32Constant(Context::SlotOffset(Context::PREVIOUS_INDEX)), - NodeProperties::GetEffectInput(node))); + NodeProperties::GetEffectInput(node), graph()->start())); } node->ReplaceInput(2, NodeProperties::GetValueInput(node, 1)); node->ReplaceInput( diff --git a/src/compiler/js-typed-lowering.cc b/src/compiler/js-typed-lowering.cc index 4f4f5d3..749fee5 100644 --- a/src/compiler/js-typed-lowering.cc +++ b/src/compiler/js-typed-lowering.cc @@ -557,10 +557,10 @@ Reduction JSTypedLowering::ReduceJSLoadProperty(Node* node) { DCHECK(IsFixedTypedArrayElementsKind(elements_kind)); element_access = AccessBuilder::ForTypedArrayElement(type, false); } - Node* value = - graph()->NewNode(simplified()->LoadElement(element_access), elements, - key, jsgraph()->Uint32Constant(length), - NodeProperties::GetEffectInput(node)); + Node* value = graph()->NewNode( + simplified()->LoadElement(element_access), elements, key, + jsgraph()->Uint32Constant(length), NodeProperties::GetEffectInput(node), + NodeProperties::GetControlInput(node)); return ReplaceEagerly(node, value); } return NoChange(); diff --git a/src/compiler/opcodes.h b/src/compiler/opcodes.h index 2d9819c..c1de689 100644 --- a/src/compiler/opcodes.h +++ b/src/compiler/opcodes.h @@ -34,7 +34,6 @@ #define INNER_OP_LIST(V) \ V(Phi) \ V(EffectPhi) \ - V(ControlEffect) \ V(ValueEffect) \ V(Finish) \ V(FrameState) \ diff --git a/src/compiler/operator-properties-inl.h b/src/compiler/operator-properties-inl.h index 51d43d4..1c9ef00 100644 --- a/src/compiler/operator-properties-inl.h +++ b/src/compiler/operator-properties-inl.h @@ -116,7 +116,8 @@ inline int OperatorProperties::GetControlInputCount(const Operator* op) { switch (op->opcode()) { case IrOpcode::kPhi: case IrOpcode::kEffectPhi: - case IrOpcode::kControlEffect: + case IrOpcode::kLoad: + case IrOpcode::kLoadElement: return 1; #define OPCODE_CASE(x) case IrOpcode::k##x: CONTROL_OP_LIST(OPCODE_CASE) @@ -149,7 +150,6 @@ inline bool OperatorProperties::HasValueOutput(const Operator* op) { inline bool OperatorProperties::HasEffectOutput(const Operator* op) { return op->opcode() == IrOpcode::kStart || - op->opcode() == IrOpcode::kControlEffect || op->opcode() == IrOpcode::kValueEffect || (op->opcode() != IrOpcode::kFinish && GetEffectInputCount(op) > 0); } diff --git a/src/compiler/simplified-lowering.cc b/src/compiler/simplified-lowering.cc index c470989..75fff31 100644 --- a/src/compiler/simplified-lowering.cc +++ b/src/compiler/simplified-lowering.cc @@ -841,6 +841,7 @@ void SimplifiedLowering::DoLoadField(Node* node) { node->set_op(machine()->Load(access.machine_type)); Node* offset = jsgraph()->Int32Constant(access.offset - access.tag()); node->InsertInput(zone(), 1, offset); + node->AppendInput(zone(), graph()->start()); } diff --git a/src/compiler/typer.cc b/src/compiler/typer.cc index 8be542e..9720798 100644 --- a/src/compiler/typer.cc +++ b/src/compiler/typer.cc @@ -309,12 +309,6 @@ Bounds Typer::Visitor::TypeEffectPhi(Node* node) { } -Bounds Typer::Visitor::TypeControlEffect(Node* node) { - UNREACHABLE(); - return Bounds(); -} - - Bounds Typer::Visitor::TypeValueEffect(Node* node) { UNREACHABLE(); return Bounds(); diff --git a/test/cctest/compiler/test-changes-lowering.cc b/test/cctest/compiler/test-changes-lowering.cc index 06308a0..3d06b90 100644 --- a/test/cctest/compiler/test-changes-lowering.cc +++ b/test/cctest/compiler/test-changes-lowering.cc @@ -132,9 +132,9 @@ class ChangesLoweringTester : public GraphBuilderTester { void* location) { // We build a graph by hand here, because the raw machine assembler // does not add the correct control and effect nodes. - Node* load = - this->graph()->NewNode(load_op, this->PointerConstant(location), - this->Int32Constant(0), this->start()); + Node* load = this->graph()->NewNode( + load_op, this->PointerConstant(location), this->Int32Constant(0), + this->start(), this->start()); Node* change = this->graph()->NewNode(op, load); Node* ret = this->graph()->NewNode(this->common()->Return(), change, this->start(), this->start()); diff --git a/test/cctest/compiler/test-simplified-lowering.cc b/test/cctest/compiler/test-simplified-lowering.cc index e403436..5cbc2d9 100644 --- a/test/cctest/compiler/test-simplified-lowering.cc +++ b/test/cctest/compiler/test-simplified-lowering.cc @@ -1355,7 +1355,7 @@ TEST(LowerLoadElement_to_load) { Node* load = t.graph()->NewNode(t.simplified()->LoadElement(access), t.p0, t.p1, - t.jsgraph.Int32Constant(1024), t.start); + t.jsgraph.Int32Constant(1024), t.start, t.start); Node* use = t.Use(load, machine_reps[i]); t.Return(use); t.Lower(); @@ -1405,7 +1405,7 @@ TEST(InsertChangeForLoadElementIndex) { kMachAnyTagged}; Node* load = t.graph()->NewNode(t.simplified()->LoadElement(access), t.p0, - t.p1, t.p2, t.start); + t.p1, t.p2, t.start, t.start); t.Return(load); t.Lower(); CHECK_EQ(IrOpcode::kLoad, load->opcode()); @@ -1445,7 +1445,7 @@ TEST(InsertChangeForLoadElement) { kMachFloat64}; Node* load = t.graph()->NewNode(t.simplified()->LoadElement(access), t.p0, - t.p1, t.p1, t.start); + t.p1, t.p1, t.start, t.start); t.Return(load); t.Lower(); CHECK_EQ(IrOpcode::kLoad, load->opcode()); diff --git a/test/unittests/compiler/change-lowering-unittest.cc b/test/unittests/compiler/change-lowering-unittest.cc index c135def..d217f77 100644 --- a/test/unittests/compiler/change-lowering-unittest.cc +++ b/test/unittests/compiler/change-lowering-unittest.cc @@ -87,6 +87,12 @@ class ChangeLoweringTest : public GraphTest { IsInt32Constant(0), IsNumberConstant(0.0), effect_matcher, control_matcher); } + Matcher IsLoadHeapNumber(const Matcher& value_matcher, + const Matcher& control_matcher) { + return IsLoad(kMachFloat64, value_matcher, + IsInt32Constant(HeapNumberValueOffset()), graph()->start(), + control_matcher); + } Matcher IsWordEqual(const Matcher& lhs_matcher, const Matcher& rhs_matcher) { return Is32() ? IsWord32Equal(lhs_matcher, rhs_matcher) @@ -226,9 +232,7 @@ TARGET_TEST_F(ChangeLowering32Test, ChangeTaggedToFloat64) { EXPECT_THAT( phi, IsPhi( - kMachFloat64, - IsLoad(kMachFloat64, val, IsInt32Constant(HeapNumberValueOffset()), - IsControlEffect(CaptureEq(&if_true))), + kMachFloat64, IsLoadHeapNumber(val, CaptureEq(&if_true)), IsChangeInt32ToFloat64( IsWord32Sar(val, IsInt32Constant(SmiShiftAmount()))), IsMerge( @@ -255,9 +259,7 @@ TARGET_TEST_F(ChangeLowering32Test, ChangeTaggedToInt32) { EXPECT_THAT( phi, IsPhi(kMachInt32, - IsChangeFloat64ToInt32(IsLoad( - kMachFloat64, val, IsInt32Constant(HeapNumberValueOffset()), - IsControlEffect(CaptureEq(&if_true)))), + IsChangeFloat64ToInt32(IsLoadHeapNumber(val, CaptureEq(&if_true))), IsWord32Sar(val, IsInt32Constant(SmiShiftAmount())), IsMerge(AllOf(CaptureEq(&if_true), IsIfTrue(CaptureEq(&branch))), IsIfFalse(AllOf( @@ -281,9 +283,7 @@ TARGET_TEST_F(ChangeLowering32Test, ChangeTaggedToUint32) { EXPECT_THAT( phi, IsPhi(kMachUint32, - IsChangeFloat64ToUint32(IsLoad( - kMachFloat64, val, IsInt32Constant(HeapNumberValueOffset()), - IsControlEffect(CaptureEq(&if_true)))), + IsChangeFloat64ToUint32(IsLoadHeapNumber(val, CaptureEq(&if_true))), IsWord32Sar(val, IsInt32Constant(SmiShiftAmount())), IsMerge(AllOf(CaptureEq(&if_true), IsIfTrue(CaptureEq(&branch))), IsIfFalse(AllOf( @@ -363,9 +363,7 @@ TARGET_TEST_F(ChangeLowering64Test, ChangeTaggedToFloat64) { EXPECT_THAT( phi, IsPhi( - kMachFloat64, - IsLoad(kMachFloat64, val, IsInt32Constant(HeapNumberValueOffset()), - IsControlEffect(CaptureEq(&if_true))), + kMachFloat64, IsLoadHeapNumber(val, CaptureEq(&if_true)), IsChangeInt32ToFloat64(IsTruncateInt64ToInt32( IsWord64Sar(val, IsInt32Constant(SmiShiftAmount())))), IsMerge( @@ -392,9 +390,7 @@ TARGET_TEST_F(ChangeLowering64Test, ChangeTaggedToInt32) { EXPECT_THAT( phi, IsPhi(kMachInt32, - IsChangeFloat64ToInt32(IsLoad( - kMachFloat64, val, IsInt32Constant(HeapNumberValueOffset()), - IsControlEffect(CaptureEq(&if_true)))), + IsChangeFloat64ToInt32(IsLoadHeapNumber(val, CaptureEq(&if_true))), IsTruncateInt64ToInt32( IsWord64Sar(val, IsInt32Constant(SmiShiftAmount()))), IsMerge(AllOf(CaptureEq(&if_true), IsIfTrue(CaptureEq(&branch))), @@ -419,9 +415,7 @@ TARGET_TEST_F(ChangeLowering64Test, ChangeTaggedToUint32) { EXPECT_THAT( phi, IsPhi(kMachUint32, - IsChangeFloat64ToUint32(IsLoad( - kMachFloat64, val, IsInt32Constant(HeapNumberValueOffset()), - IsControlEffect(CaptureEq(&if_true)))), + IsChangeFloat64ToUint32(IsLoadHeapNumber(val, CaptureEq(&if_true))), IsTruncateInt64ToInt32( IsWord64Sar(val, IsInt32Constant(SmiShiftAmount()))), IsMerge(AllOf(CaptureEq(&if_true), IsIfTrue(CaptureEq(&branch))), diff --git a/test/unittests/compiler/common-operator-unittest.cc b/test/unittests/compiler/common-operator-unittest.cc index 571d696..2cb4d19 100644 --- a/test/unittests/compiler/common-operator-unittest.cc +++ b/test/unittests/compiler/common-operator-unittest.cc @@ -51,8 +51,7 @@ const SharedOperator kSharedOperators[] = { SHARED(IfTrue, Operator::kFoldable, 0, 0, 1, 0, 1), SHARED(IfFalse, Operator::kFoldable, 0, 0, 1, 0, 1), SHARED(Throw, Operator::kFoldable, 1, 0, 1, 0, 1), - SHARED(Return, Operator::kNoProperties, 1, 1, 1, 1, 1), - SHARED(ControlEffect, Operator::kPure, 0, 0, 1, 1, 0) + SHARED(Return, Operator::kNoProperties, 1, 1, 1, 1, 1) #undef SHARED }; diff --git a/test/unittests/compiler/graph-unittest.cc b/test/unittests/compiler/graph-unittest.cc index ee00889..5160e9a 100644 --- a/test/unittests/compiler/graph-unittest.cc +++ b/test/unittests/compiler/graph-unittest.cc @@ -435,12 +435,14 @@ class IsLoadMatcher FINAL : public NodeMatcher { IsLoadMatcher(const Matcher& rep_matcher, const Matcher& base_matcher, const Matcher& index_matcher, - const Matcher& effect_matcher) + const Matcher& effect_matcher, + const Matcher& control_matcher) : NodeMatcher(IrOpcode::kLoad), rep_matcher_(rep_matcher), base_matcher_(base_matcher), index_matcher_(index_matcher), - effect_matcher_(effect_matcher) {} + effect_matcher_(effect_matcher), + control_matcher_(control_matcher) {} virtual void DescribeTo(std::ostream* os) const OVERRIDE { NodeMatcher::DescribeTo(os); @@ -450,8 +452,10 @@ class IsLoadMatcher FINAL : public NodeMatcher { base_matcher_.DescribeTo(os); *os << "), index ("; index_matcher_.DescribeTo(os); - *os << ") and effect ("; + *os << "), effect ("; effect_matcher_.DescribeTo(os); + *os << ") and control ("; + control_matcher_.DescribeTo(os); *os << ")"; } @@ -465,7 +469,9 @@ class IsLoadMatcher FINAL : public NodeMatcher { PrintMatchAndExplain(NodeProperties::GetValueInput(node, 1), "index", index_matcher_, listener) && PrintMatchAndExplain(NodeProperties::GetEffectInput(node), "effect", - effect_matcher_, listener)); + effect_matcher_, listener) && + PrintMatchAndExplain(NodeProperties::GetControlInput(node), + "control", control_matcher_, listener)); } private: @@ -473,6 +479,7 @@ class IsLoadMatcher FINAL : public NodeMatcher { const Matcher base_matcher_; const Matcher index_matcher_; const Matcher effect_matcher_; + const Matcher control_matcher_; }; @@ -625,12 +632,6 @@ Matcher IsIfFalse(const Matcher& control_matcher) { } -Matcher IsControlEffect(const Matcher& control_matcher) { - return MakeMatcher( - new IsControl1Matcher(IrOpcode::kControlEffect, control_matcher)); -} - - Matcher IsValueEffect(const Matcher& value_matcher) { return MakeMatcher(new IsUnopMatcher(IrOpcode::kValueEffect, value_matcher)); } @@ -717,9 +718,10 @@ Matcher IsCall(const Matcher& descriptor_matcher, Matcher IsLoad(const Matcher& rep_matcher, const Matcher& base_matcher, const Matcher& index_matcher, - const Matcher& effect_matcher) { + const Matcher& effect_matcher, + const Matcher& control_matcher) { return MakeMatcher(new IsLoadMatcher(rep_matcher, base_matcher, index_matcher, - effect_matcher)); + effect_matcher, control_matcher)); } diff --git a/test/unittests/compiler/graph-unittest.h b/test/unittests/compiler/graph-unittest.h index c59c669..18d3ca5 100644 --- a/test/unittests/compiler/graph-unittest.h +++ b/test/unittests/compiler/graph-unittest.h @@ -58,7 +58,6 @@ 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 IsValueEffect(const Matcher& value_matcher); Matcher IsFinish(const Matcher& value_matcher, const Matcher& effect_matcher); @@ -93,7 +92,8 @@ Matcher IsNumberSubtract(const Matcher& lhs_matcher, Matcher IsLoad(const Matcher& rep_matcher, const Matcher& base_matcher, const Matcher& index_matcher, - const Matcher& effect_matcher); + const Matcher& effect_matcher, + const Matcher& control_matcher); Matcher IsStore(const Matcher& type_matcher, const Matcher& write_barrier_matcher, const Matcher& base_matcher, diff --git a/test/unittests/compiler/machine-operator-unittest.cc b/test/unittests/compiler/machine-operator-unittest.cc index db73a81..a417a4b 100644 --- a/test/unittests/compiler/machine-operator-unittest.cc +++ b/test/unittests/compiler/machine-operator-unittest.cc @@ -59,8 +59,8 @@ TEST_P(MachineLoadOperatorTest, NumberOfInputsAndOutputs) { EXPECT_EQ(2, OperatorProperties::GetValueInputCount(op)); EXPECT_EQ(1, OperatorProperties::GetEffectInputCount(op)); - EXPECT_EQ(0, OperatorProperties::GetControlInputCount(op)); - EXPECT_EQ(3, OperatorProperties::GetTotalInputCount(op)); + EXPECT_EQ(1, OperatorProperties::GetControlInputCount(op)); + EXPECT_EQ(4, OperatorProperties::GetTotalInputCount(op)); EXPECT_EQ(1, OperatorProperties::GetValueOutputCount(op)); EXPECT_EQ(1, OperatorProperties::GetEffectOutputCount(op)); diff --git a/test/unittests/compiler/simplified-operator-unittest.cc b/test/unittests/compiler/simplified-operator-unittest.cc index dcabc1a..8c5a918 100644 --- a/test/unittests/compiler/simplified-operator-unittest.cc +++ b/test/unittests/compiler/simplified-operator-unittest.cc @@ -177,8 +177,8 @@ TEST_P(SimplifiedElementAccessOperatorTest, LoadElement) { EXPECT_EQ(3, OperatorProperties::GetValueInputCount(op)); EXPECT_EQ(1, OperatorProperties::GetEffectInputCount(op)); - EXPECT_EQ(0, OperatorProperties::GetControlInputCount(op)); - EXPECT_EQ(4, OperatorProperties::GetTotalInputCount(op)); + EXPECT_EQ(1, OperatorProperties::GetControlInputCount(op)); + EXPECT_EQ(5, OperatorProperties::GetTotalInputCount(op)); EXPECT_EQ(1, OperatorProperties::GetValueOutputCount(op)); EXPECT_EQ(1, OperatorProperties::GetEffectOutputCount(op)); -- 2.7.4