[PATCH] [LoopReroll] Introduce the concept of DAGRootSets.

hfinkel at anl.gov hfinkel at anl.gov
Fri Feb 6 13:53:12 PST 2015


REPOSITORY
  rL LLVM

================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:346
@@ +345,3 @@
+    //   x[i*2+5] = ...   (2)
+    //   x[(i+1234)*2+5678] = ... (3)
+    //   x[(i+1234)*2+5679] = ... (3)
----------------
Please also explain here how the loop will be rerolled (by adding multiple induction variables to the rerolled loop, one for each DAGRootSet).

================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:622
@@ -588,6 +621,3 @@
 
-bool LoopReroll::DAGRootTracker::findRoots() {
-
-  const SCEVAddRecExpr *RealIVSCEV = cast<SCEVAddRecExpr>(SE->getSCEV(IV));
-  Inc = cast<SCEVConstant>(RealIVSCEV->getOperand(1))->
-    getValue()->getZExtValue();
+static bool isSimpleArithmeticOp(User *IVU) {
+  if (Instruction *I = dyn_cast<Instruction>(IVU)) {
----------------
Please add a comment here explaining how this is used (which I gather is to prune the search space when looking for arithmetic expressions on the induction variables).

================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:699
@@ +698,3 @@
+    // FIXME: Do negative values "just work"? or do we have to invert some
+    // logic somewhere?
+    if (V < 0) {
----------------
IIRC, you need to adjust where the loop starts once you reroll it (and how validation works, etc. because your iteration space now starts at some negative number instead of zero).

================
Comment at: lib/Transforms/Scalar/LoopRerollPass.cpp:848
@@ +847,3 @@
+    const SCEV *StepSCEV = SE->getMinusSCEV(SE->getSCEV(V.Roots[0]), ADR);
+    if (ADR->getPostIncExpr(*SE) != SE->getAddExpr(LastSCEV, StepSCEV)) {
+      DEBUG(dbgs() << "LRR: Aborting because iterations are not consecutive\n");
----------------
Please add a comment explaining the algebra here.

You're comparing:
  ADR_next == Roots[Last] + (Roots[0] - ADR)
which is:
  ADR + ADR_step == Roots[Last] + Roots[0] - ADR
which is:
 2*ADR + ADR_step == Roots[Last] + Roots[0]
which I don't understand.

Why don't you compare ADR->getStepRecurrence() to SE->getMinusSCEV(SE->getSCEV(V.Roots[LastIdx]), SE->getSCEV(V.Roots[0])) (or something like that)?

http://reviews.llvm.org/D7439

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list