[turbofan] Add control input to Load and LoadElements.
authorbmeurer@chromium.org <bmeurer@chromium.org>
Wed, 1 Oct 2014 11:08:37 +0000 (11:08 +0000)
committerbmeurer@chromium.org <bmeurer@chromium.org>
Wed, 1 Oct 2014 11:08:37 +0000 (11:08 +0000)
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

18 files changed:
src/compiler/ast-graph-builder.cc
src/compiler/change-lowering.cc
src/compiler/common-operator.cc
src/compiler/common-operator.h
src/compiler/js-generic-lowering.cc
src/compiler/js-typed-lowering.cc
src/compiler/opcodes.h
src/compiler/operator-properties-inl.h
src/compiler/simplified-lowering.cc
src/compiler/typer.cc
test/cctest/compiler/test-changes-lowering.cc
test/cctest/compiler/test-simplified-lowering.cc
test/unittests/compiler/change-lowering-unittest.cc
test/unittests/compiler/common-operator-unittest.cc
test/unittests/compiler/graph-unittest.cc
test/unittests/compiler/graph-unittest.h
test/unittests/compiler/machine-operator-unittest.cc
test/unittests/compiler/simplified-operator-unittest.cc

index d33b05a..3ac4b46 100644 (file)
@@ -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;
index 5b66d55..d1cbdb8 100644 (file)
@@ -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);
 }
 
 
index 4ce98aa..f0f158b 100644 (file)
@@ -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,
index 2e30a04..941fb17 100644 (file)
@@ -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);
index 3708c7f..5affb77 100644 (file)
@@ -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<int>(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(
index 4f4f5d3..749fee5 100644 (file)
@@ -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();
index 2d9819c..c1de689 100644 (file)
@@ -34,7 +34,6 @@
 #define INNER_OP_LIST(V) \
   V(Phi)                 \
   V(EffectPhi)           \
-  V(ControlEffect)       \
   V(ValueEffect)         \
   V(Finish)              \
   V(FrameState)          \
index 51d43d4..1c9ef00 100644 (file)
@@ -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);
 }
index c470989..75fff31 100644 (file)
@@ -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());
 }
 
 
index 8be542e..9720798 100644 (file)
@@ -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();
index 06308a0..3d06b90 100644 (file)
@@ -132,9 +132,9 @@ class ChangesLoweringTester : public GraphBuilderTester<ReturnType> {
                          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());
index e403436..5cbc2d9 100644 (file)
@@ -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());
index c135def..d217f77 100644 (file)
@@ -87,6 +87,12 @@ class ChangeLoweringTest : public GraphTest {
         IsInt32Constant(0), IsNumberConstant(0.0), effect_matcher,
         control_matcher);
   }
+  Matcher<Node*> IsLoadHeapNumber(const Matcher<Node*>& value_matcher,
+                                  const Matcher<Node*>& control_matcher) {
+    return IsLoad(kMachFloat64, value_matcher,
+                  IsInt32Constant(HeapNumberValueOffset()), graph()->start(),
+                  control_matcher);
+  }
   Matcher<Node*> IsWordEqual(const Matcher<Node*>& lhs_matcher,
                              const Matcher<Node*>& 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))),
index 571d696..2cb4d19 100644 (file)
@@ -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
 };
 
index ee00889..5160e9a 100644 (file)
@@ -435,12 +435,14 @@ class IsLoadMatcher FINAL : public NodeMatcher {
   IsLoadMatcher(const Matcher<LoadRepresentation>& rep_matcher,
                 const Matcher<Node*>& base_matcher,
                 const Matcher<Node*>& index_matcher,
-                const Matcher<Node*>& effect_matcher)
+                const Matcher<Node*>& effect_matcher,
+                const Matcher<Node*>& 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<Node*> base_matcher_;
   const Matcher<Node*> index_matcher_;
   const Matcher<Node*> effect_matcher_;
+  const Matcher<Node*> control_matcher_;
 };
 
 
@@ -625,12 +632,6 @@ Matcher<Node*> IsIfFalse(const Matcher<Node*>& control_matcher) {
 }
 
 
-Matcher<Node*> IsControlEffect(const Matcher<Node*>& control_matcher) {
-  return MakeMatcher(
-      new IsControl1Matcher(IrOpcode::kControlEffect, control_matcher));
-}
-
-
 Matcher<Node*> IsValueEffect(const Matcher<Node*>& value_matcher) {
   return MakeMatcher(new IsUnopMatcher(IrOpcode::kValueEffect, value_matcher));
 }
@@ -717,9 +718,10 @@ Matcher<Node*> IsCall(const Matcher<CallDescriptor*>& descriptor_matcher,
 Matcher<Node*> IsLoad(const Matcher<LoadRepresentation>& rep_matcher,
                       const Matcher<Node*>& base_matcher,
                       const Matcher<Node*>& index_matcher,
-                      const Matcher<Node*>& effect_matcher) {
+                      const Matcher<Node*>& effect_matcher,
+                      const Matcher<Node*>& control_matcher) {
   return MakeMatcher(new IsLoadMatcher(rep_matcher, base_matcher, index_matcher,
-                                       effect_matcher));
+                                       effect_matcher, control_matcher));
 }
 
 
index c59c669..18d3ca5 100644 (file)
@@ -58,7 +58,6 @@ Matcher<Node*> IsMerge(const Matcher<Node*>& control0_matcher,
                        const Matcher<Node*>& control1_matcher);
 Matcher<Node*> IsIfTrue(const Matcher<Node*>& control_matcher);
 Matcher<Node*> IsIfFalse(const Matcher<Node*>& control_matcher);
-Matcher<Node*> IsControlEffect(const Matcher<Node*>& control_matcher);
 Matcher<Node*> IsValueEffect(const Matcher<Node*>& value_matcher);
 Matcher<Node*> IsFinish(const Matcher<Node*>& value_matcher,
                         const Matcher<Node*>& effect_matcher);
@@ -93,7 +92,8 @@ Matcher<Node*> IsNumberSubtract(const Matcher<Node*>& lhs_matcher,
 Matcher<Node*> IsLoad(const Matcher<LoadRepresentation>& rep_matcher,
                       const Matcher<Node*>& base_matcher,
                       const Matcher<Node*>& index_matcher,
-                      const Matcher<Node*>& effect_matcher);
+                      const Matcher<Node*>& effect_matcher,
+                      const Matcher<Node*>& control_matcher);
 Matcher<Node*> IsStore(const Matcher<MachineType>& type_matcher,
                        const Matcher<WriteBarrierKind>& write_barrier_matcher,
                        const Matcher<Node*>& base_matcher,
index db73a81..a417a4b 100644 (file)
@@ -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));
index dcabc1a..8c5a918 100644 (file)
@@ -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));