[PATCH] Reimplement heuristic for estimating complete-unroll optimization effects.

Chandler Carruth chandlerc at gmail.com
Mon Feb 23 19:51:48 PST 2015


A few more code simplification comments just so you have them queued up.


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:518
@@ +517,3 @@
+                    dyn_cast_or_null<ConstantInt>(SimplifiedValues.lookup(Cond))) {
+              CertainSucc = BI->getSuccessor(SimpleCond->isZero() ? 1 : 0);
+            }
----------------
Rather than keeping a variable, just insert this onto the worklist an 'continue'? (see below for why this might be better)

================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:521
@@ -447,1 +520,3 @@
+          } else {
+            CertainSucc = BI->getSuccessor(0);
           }
----------------
No need to handle this, just fall through two the successor loop.

================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:527
@@ +526,3 @@
+                  dyn_cast_or_null<ConstantInt>(SimplifiedValues.lookup(Cond))) {
+            CertainSucc = SI->findCaseValue(SimpleCond).getCaseSuccessor();
+          }
----------------
Just insert and continue here as well? (again, see below for why this might be ok)

================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:536
@@ +535,3 @@
+          else
+            EarlyExitFound = true;
+        } else {
----------------
Ok, I see what you're trying to do here. You're trying to check for us *definitely* exiting the loop after N iterations due to branching to the exit block.

But I don't think this code is correct at this point. While you know that if this basic block is dynamically executed on this iteration, the loop will be exited, you don't know that this block will be dynamically executed on this iteration.

This only seems important to handle cases where simulating N iterations proves something about the trip count that SCEV can't prove, which on the whole seems like a bug in SCEV. Is it really worth trying to handle it here? If so, you'll want to check that this block is the only loop exiting block or otherwise ensure that it is dynamically executed on every iteration without fail.

If you decide not to handle this, then that makes using 'continue' in my two comments above make sense.

================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:546-551
@@ -450,1 +545,8 @@
+
+      // If we found no optimization opportunities on the first iteration, we
+      // won't find them on later ones too.
+      if (!NumberOfOptimizedInstructions) {
+        UnrolledLoopSize = UINT_MAX;
+        return;
+      }
     }
----------------
Handle this before we process the terminators?

http://reviews.llvm.org/D7837

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






More information about the llvm-commits mailing list