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

Michael Zolotukhin mzolotukhin at apple.com
Mon Feb 23 20:00:51 PST 2015


Thanks for the comments, I'll post updated patch shortly.


================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:527
@@ +526,3 @@
+                  dyn_cast_or_null<ConstantInt>(SimplifiedValues.lookup(Cond))) {
+            CertainSucc = SI->findCaseValue(SimpleCond).getCaseSuccessor();
+          }
----------------
chandlerc wrote:
> Just insert and continue here as well? (again, see below for why this might be ok)
Makes sense. It's a kind of leftover from inlining addBBSuccessors.

================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:536
@@ +535,3 @@
+          else
+            EarlyExitFound = true;
+        } else {
----------------
chandlerc wrote:
> 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.
Oh, you are right. I incorrectly assumed that all blocks from worklist would be actually executed.

I think I'll just drop this then to keep the code simpler.

================
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;
+      }
     }
----------------
chandlerc wrote:
> Handle this before we process the terminators?
Makes sense.

http://reviews.llvm.org/D7837

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






More information about the llvm-commits mailing list