[PATCH] D12008: [ScalarEvolutionExpander] Refactor isHighCostExpansionHelper division case

[Sanjoy Das via llvm-commits]

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

LGTM, but please mention something along the lines of

"However this heuristic is based on the fact that we are looking at the loop exit conditions. All latches which exit loop should be listed in the ExitingBlocks. So the overall heuristic stays roughly the same."

in the commit message.


================
Comment at: lib/Analysis/ScalarEvolutionExpander.cpp:1819
@@ -1818,3 +1818,3 @@
 
-  SmallVector<BasicBlock *, 4> Latches;
-  L->getLoopLatches(Latches);
+  SmallVector<BasicBlock *, 4> ExitingBlocks;
+  L->getExitingBlocks(ExitingBlocks);
----------------
igor-laevsky wrote:
> sanjoy wrote:
> > Latches are not exiting blocks, so this is a semantic change.  Perhaps this should be iterating over both exits and latches?
> Right. However this heuristic is based on the fact that we are looking at the loop exit conditions. All latches which exit loop should be listed in the ExitingBlocks. So the overall heuristic stays roughly the same. I probably shouldn't call this change purely a refactoring though.
> 
> If you think that it is important to visit non-exiting latches it would be easy to add them into lookup list. It's just small amount of additional complexity which I don't see real need for.
Ok.


Repository:
  rL LLVM

http://reviews.llvm.org/D12008