[turbofan] First version of loop peeling.

test/unittests/compiler/graph-unittest.h

Index: test/unittests/compiler/graph-unittest.h
diff --git a/test/unittests/compiler/graph-unittest.h b/test/unittests/compiler/graph-unittest.h
index 7c75161d9d2c092624fe3b247d5808476594f52c..c7285a042e1509fb6bdb634aa60f2e412d813ee9 100644
--- a/test/unittests/compiler/graph-unittest.h
+++ b/test/unittests/compiler/graph-unittest.h
@@ -7,6 +7,8 @@
 
 #include "src/compiler/common-operator.h"
 #include "src/compiler/graph.h"
+#include "src/compiler/machine-operator.h"
+#include "src/compiler/node.h"
 #include "src/compiler/typer.h"
 #include "test/unittests/test-utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -31,7 +33,6 @@ class GraphTest : public TestWithContext, public TestWithZone {
   explicit GraphTest(int num_parameters = 1);
   ~GraphTest() OVERRIDE;
 
- protected:
   Node* Parameter(int32_t index = 0);
   Node* Float32Constant(volatile float value);
   Node* Float64Constant(volatile double value);
@@ -50,6 +51,7 @@ class GraphTest : public TestWithContext, public TestWithZone {
   Matcher<Node*> IsFalseConstant();
   Matcher<Node*> IsTrueConstant();
 
+  Node* start() { return graph()->start(); }
   CommonOperatorBuilder* common() { return &common_; }
   Graph* graph() { return &graph_; }
 
@@ -64,7 +66,6 @@ class TypedGraphTest : public GraphTest {
   explicit TypedGraphTest(int num_parameters = 1);
   ~TypedGraphTest() OVERRIDE;
 
- protected:
   Node* Parameter(int32_t index = 0) { return GraphTest::Parameter(index); }
   Node* Parameter(Type* type, int32_t index = 0);
 
@@ -74,6 +75,76 @@ class TypedGraphTest : public GraphTest {
   Typer typer_;
 };
 
+

[Benedikt Meurer, 2015/01/15 13:16:16]

Move these helpers to loop-peeling-unittest.cc. Also they should not depend on
GraphTest, but rather take a JSGraph.

[titzer, 2015/01/15 13:36:51]

The plan is to use them in other tests, so maybe another header is more
appropriate, but atm I don't think we are suffering from the too-few header
files problem.

And, they don't really need a JSGraph, just a Graph* and a
CommonOperatorBuilder*, which GraphTest has.

[Benedikt Meurer, 2015/01/15 13:43:09]

Well this kind of goes against the purpose of GraphTest. GraphTest should be a
base class for Graph tests and as independent and generic as possible. So please
move these to loop-peeling-unittest.cc and only hoist them into a header when
really necessary. And if you only need a Graph and CommonOperatorBuilder, then
pass them as parameters. GraphTest should not be used as a way to pass around
Graph+CommonOperatorBuilder.

[titzer, 2015/01/16 16:15:37]

PTAL, I've made these utilities into standalone structs and added helper methods
to the LoopPeelerTest to create and chain them together.

[Benedikt Meurer, 2015/01/16 17:55:36]

Thanks. Now you can undo the changes to graph-unittest.h as they're no longer
needed.

[titzer, 2015/01/19 09:47:11]

Done.

+// A helper for building while loops.
+struct While {

[Benedikt Meurer, 2015/01/15 13:16:16]

According to the style guide "structs should be used for passive objects that
carry data, and may have associated constants, but lack any functionality other
than access/setting the data members. The accessing/setting of fields is done by
directly accessing the fields rather than through method invocations. Methods
should not provide behavior but should only be used to set up the data members,
e.g., constructor, destructor, Initialize(), Reset(), Validate().", so this has
to be class then. See
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Structs_vs._C...

[titzer, 2015/01/15 13:36:51]

I'd prefer to keep these as structs to avoid accessors for all the members, so
that means what? Moving Chain() and Nest() and Phi() to somewhere else?

[Benedikt Meurer, 2015/01/15 13:43:09]

Personally I don't like these helper structs at all, esp. not in header files.
IMHO it's easier to read the code without these helpers, even if the code is
twice as long, but then you don't need to lookup/remember the helper magic. The
Diamond class already caused quite some trouble.

But I'm fine with these as long as they are local within the .cc file, ideally
within the class that they are built for.

+  GraphTest* t;
+  Node* branch;
+  Node* if_true;
+  Node* exit;
+  Node* loop;
+
+  While(GraphTest* R, Node* cond, Node* control = nullptr) : t(R) {
+    if (control == nullptr) control = R->start();
+    loop = t->graph()->NewNode(t->common()->Loop(2), control, control);
+    branch = t->graph()->NewNode(t->common()->Branch(), cond, loop);
+    if_true = t->graph()->NewNode(t->common()->IfTrue(), branch);
+    exit = t->graph()->NewNode(t->common()->IfFalse(), branch);
+    loop->ReplaceInput(1, if_true);
+  }
+
+  void Chain(Node* control) { loop->ReplaceInput(0, control); }
+  void Nest(While* that) {
+    that->loop->ReplaceInput(1, exit);
+    this->loop->ReplaceInput(0, that->if_true);
+  }
+  Node* Phi(Node* a, Node* b) {
+    return t->graph()->NewNode(t->common()->Phi(kMachAnyTagged, 2), a, b, loop);
+  }
+};
+
+
+// A helper for building branches.
+struct Branch {

[Benedikt Meurer, 2015/01/15 13:16:16]

See above.

+  Node* branch;
+  Node* if_true;
+  Node* if_false;
+
+  Branch(GraphTest* R, Node* cond, Node* control = nullptr) {
+    if (control == nullptr) control = R->start();
+    branch = R->graph()->NewNode(R->common()->Branch(), cond, control);
+    if_true = R->graph()->NewNode(R->common()->IfTrue(), branch);
+    if_false = R->graph()->NewNode(R->common()->IfFalse(), branch);
+  }
+};
+
+
+// A helper for building counters attached to loops.
+struct Counter {

[Benedikt Meurer, 2015/01/15 13:16:16]

See above.

+  Node* base;
+  Node* inc;
+  Node* phi;
+  Node* add;
+
+  Counter(While& w, MachineOperatorBuilder* machine, int32_t b, int32_t k)

[Benedikt Meurer, 2015/01/15 13:16:16]

According to the google style guide: All parameters passed by reference must be
labeled const.

[titzer, 2015/01/15 13:36:51]

Done.

+      : base(w.t->Int32Constant(b)), inc(w.t->Int32Constant(k)) {
+    Build(w, machine);
+  }
+
+  Counter(While& w, MachineOperatorBuilder* machine, Node* b, Node* k)

[Benedikt Meurer, 2015/01/15 13:16:16]

According to the google style guide: All parameters passed by reference must be
labeled const.

[titzer, 2015/01/15 13:36:51]

Done.

+      : base(b), inc(k) {
+    Build(w, machine);
+  }
+
+  void Build(While& w, MachineOperatorBuilder* machine) {

[Benedikt Meurer, 2015/01/15 13:16:16]

According to the google style guide: All parameters passed by reference must be
labeled const.

[titzer, 2015/01/15 13:36:51]

Done.

+    phi = w.t->graph()->NewNode(w.t->common()->Phi(kMachAnyTagged, 2), base,
+                                base, w.loop);
+    add = w.t->graph()->NewNode(machine->Int32Add(), phi, inc);
+    phi->ReplaceInput(1, add);
+  }
+};
+
+
 }  //  namespace compiler
 }  //  namespace internal
 }  //  namespace v8