[PATCH] D12428: Traverse the SCoP to compute non-loop-carried domain conditions

[Tobias Grosser via llvm-commits]

grosser accepted this revision.
grosser added a comment.
This revision is now accepted and ready to land.

LGTM, only some minor typos.


================
Comment at: include/polly/ScopInfo.h:914
@@ +913,3 @@
+  /// @brief Compute the branching constraints for each basic block in @p R.
+  ///
+  /// @param R  The region we currently build branching conditions for.
----------------
@brief

================
Comment at: include/polly/ScopInfo.h:916
@@ +915,3 @@
+  /// @param R  The region we currently build branching conditions for.
+  /// @param LI The LoopInfo analysis to argue about the number of iterators.
+  /// @param SD The ScopDetection analysis to identify non-affine sub-regions.
----------------
'to argue'? maybe 'to question' or 'to obtain'?

================
Comment at: include/polly/ScopInfo.h:918
@@ +917,3 @@
+  /// @param SD The ScopDetection analysis to identify non-affine sub-regions.
+  /// @param DT The dominator tree of the current function.  void
+  void buildDomainsWithBranchConstraints(Region *R, LoopInfo &LI,
----------------
'void' at the end of the line?

================
Comment at: lib/Analysis/ScopInfo.cpp:817
@@ -743,2 +816,3 @@
 
-  unsigned loopDimension = getParent()->getRelativeLoopDepth(L);
+  int relativeLoopDimension = getParent()->getRelativeLoopDepth(L);
+  assert(relativeLoopDimension >= 0 &&
----------------
Uppercase.

================
Comment at: lib/Analysis/ScopInfo.cpp:820
@@ +819,3 @@
+         "Expected relative loop depth of L to be non-negative");
+  unsigned loopDimension = relativeLoopDimension;
+
----------------
Uppercase.

================
Comment at: lib/Analysis/ScopInfo.cpp:1515
@@ +1514,3 @@
+  // element is executed. To this end we iterate in reverse post order over R as
+  // it ensures that we first visist all predecessors of a region node (either a
+  // basic block or a subregion) before we visit the region node itself.
----------------
visit

================
Comment at: lib/Analysis/ScopInfo.cpp:1559
@@ +1558,3 @@
+    // Now iterate over the successors and set their initial domain based on
+    // their condition set. We skip back edges here and have to be carefull when
+    // we leave a loop not to keep constraints over a dimension that doesn't
----------------
careful

================
Comment at: lib/Analysis/ScopInfo.cpp:1574
@@ +1573,3 @@
+      // constrains
+      // on the loop/dimension we leave.
+      Loop *SuccBBLoop = LI.getLoopFor(SuccBB);
----------------
why the linebreak?


http://reviews.llvm.org/D12428