[PATCH] D102982: [LoopUnroll] Use smallest exact trip count from any exit

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp:1119
+  L->getExitingBlocks(ExitingBlocks);
+  for (BasicBlock *BB : ExitingBlocks) {
+    // Only unroll against latch-dominating exit blocks.
----------------
nikic wrote:
> reames wrote:
> > I would strongly prefer this logic be sunk into the appropriate SCEV accessors.  I'm fine with you doing that in a follow up provided you commit to doing so.  I'll leave that decision up to you.  
> Could you please clarify which SCEV accessors you have in mind here? Do you mean sinking this into the getSmallConstantTripCount() variant that only accepts a Loop? I would have a couple of concerns with doing that:
> 
>  * We also need to know which exit the trip count refers to.
>  * This is not really the trip count of the loop (just a loop exit), and I'm pretty sure changing that would break other users of the API.
>  * The limitation to latch-dominating exits here is not fundamental, and mainly there due to unclear profitability.
> 
> Maybe I misunderstood the suggestion though.
I was referring specifically to the small constant trip count and small constant multiple versions which take Loop parameters.  

Your point about needing the exit block is true for the way the code is currently phrased.  I'd missed that.  I think the need for that can and should be removed (see my comment on the patch this one is based on), but if that's logistically complicated, I'm fine with us moving forward with this structure and then revisiting in the future.  

Any exit count for an exit which dominates the latch must be a (potentially conservative) exit count for the loop.  So, I'm not quite sure what you mean with the rest of your comments.  


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102982/new/

https://reviews.llvm.org/D102982