Revision: 23489
Author:   sigu...@chromium.org
Date:     Thu Aug 28 12:18:25 2014 UTC
Log:      Simplify inlining now that the scheduler is smart(er).

* Only control adjustment is to move everything from the inlinee's
  start block to the block the call was in.
* Add a unit test to ensure that the scheduler actually picks the
  right order when placing the code.

R=mstarzin...@chromium.org, tit...@chromium.org

Review URL: https://codereview.chromium.org/508953002
https://code.google.com/p/v8/source/detail?r=23489

Modified:
 /branches/bleeding_edge/src/compiler/js-inlining.cc
 /branches/bleeding_edge/test/cctest/compiler/test-run-inlining.cc

=======================================
--- /branches/bleeding_edge/src/compiler/js-inlining.cc Tue Aug 26 13:09:08 2014 UTC +++ /branches/bleeding_edge/src/compiler/js-inlining.cc Thu Aug 28 12:18:25 2014 UTC
@@ -47,35 +47,6 @@
   InlinerVisitor visitor(this);
   jsgraph_->graph()->VisitNodeInputsFromEnd(&visitor);
 }
-
-
-static void MoveWithDependencies(Graph* graph, Node* node, Node* old_block,
-                                 Node* new_block) {
-  if (OperatorProperties::HasControlInput(node->op())) {
-    // Check if we have left the old_block.
-    if (NodeProperties::GetControlInput(node) != old_block) return;
-    // If not, move this node to the new_block.
-    NodeProperties::ReplaceControlInput(node, new_block);
-  }
-  // Independent of whether a node has a control input or not,
-  // it might have a dependency that is pinned to old_block.
- for (InputIter iter = node->inputs().begin(); iter != node->inputs().end();
-       ++iter) {
-    if (NodeProperties::IsControlEdge(iter.edge())) continue;
-    MoveWithDependencies(graph, *iter, old_block, new_block);
-  }
-}
-
-
-static void MoveAllControlNodes(Node* from, Node* to) {
-  for (UseIter iter = from->uses().begin(); iter != from->uses().end();) {
-    if (NodeProperties::IsControlEdge(iter.edge())) {
-      iter.UpdateToAndIncrement(to);
-    } else {
-      ++iter;
-    }
-  }
-}


// TODO(sigurds) Find a home for this function and reuse it everywhere (esp. in
@@ -187,13 +158,9 @@
 void Inlinee::InlineAtCall(JSGraph* jsgraph, Node* call) {
   MachineOperatorBuilder machine(jsgraph->zone());

+ // The scheduler is smart enough to place our code; we just ensure {control}
+  // becomes the control input of the start of the inlinee.
   Node* control = NodeProperties::GetControlInput(call);
-  // Move all the nodes to the end block.
-  MoveAllControlNodes(control, end_block());
-  // Now move the ones the call depends on back up.
-  // We have to do this back-and-forth to treat the case where the call is
-  // pinned to the start block.
-  MoveWithDependencies(graph(), call, end_block(), control);

   // The inlinee uses the context from the JSFunction object. This will
// also be the effect dependency for the inlinee as it produces an effect.
=======================================
--- /branches/bleeding_edge/test/cctest/compiler/test-run-inlining.cc Thu Aug 28 08:39:24 2014 UTC +++ /branches/bleeding_edge/test/cctest/compiler/test-run-inlining.cc Thu Aug 28 12:18:25 2014 UTC
@@ -162,33 +162,34 @@
 TEST(InlineTwiceDependentDiamond) {
   FunctionTester T(
       "(function () {"
-      "function foo(s) { if (true) {"
-      "                  return 12 } else { return 13; } };"
-      "function bar(s,t) { return foo(foo(1)); };"
+      "var x = 41;"
+      "function foo(s) { AssertStackDepth(1); if (s % 2 == 0) {"
+      "                  return x - s } else { return x + s; } };"
+      "function bar(s,t) { return foo(foo(s)); };"
       "return bar;"
       "})();",
       CompilationInfo::kInliningEnabled |
           CompilationInfo::kContextSpecializing);

   InstallAssertStackDepthHelper(CcTest::isolate());
-  T.CheckCall(T.Val(12), T.undefined(), T.undefined());
+  T.CheckCall(T.Val(-11), T.Val(11), T.Val(4));
 }


-TEST(InlineTwiceDependentDiamondReal) {
+TEST(InlineTwiceDependentDiamondDifferent) {
   FunctionTester T(
       "(function () {"
       "var x = 41;"
-      "function foo(s) { AssertStackDepth(1); if (s % 2 == 0) {"
-      "                  return x - s } else { return x + s; } };"
-      "function bar(s,t) { return foo(foo(s)); };"
+      "function foo(s,t) { AssertStackDepth(1); if (s % 2 == 0) {"
+ " return x - s * t } else { return x + s * t; } };"
+      "function bar(s,t) { return foo(foo(s, 3), 5); };"
       "return bar;"
       "})();",
       CompilationInfo::kInliningEnabled |
           CompilationInfo::kContextSpecializing);

   InstallAssertStackDepthHelper(CcTest::isolate());
-  T.CheckCall(T.Val(-11), T.Val(11), T.Val(4));
+  T.CheckCall(T.Val(-329), T.Val(11), T.Val(4));
 }

 #endif  // V8_TURBOFAN_TARGET

--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to