[PATCH] D19553: Unroll pass restructure.

[Evgeny Stupachenko via llvm-commits]

evstupac added inline comments.

================
Comment at: lib/Transforms/Scalar/LoopUnrollPass.cpp:807
@@ +806,3 @@
+        UnrolledSize = (LoopSize - BEInsns) * UP.Count + BEInsns;
+        while (UP.Count != 0 && UnrolledSize > UP.PartialThreshold) {
+          UP.Count >>= 1;
----------------
Gerolf wrote:
> Isn't that simply UP.Count = max(UP.PartialThreshold - BEInsns/ LoopSize -BEInsns -1, 0)? Probably it also needs a check that LoopSize > BEInsns, but that should probably be an assertion anyway. And for UP.PartialThreshold - BEInsns/LoopSize - BEInsns should  probably be fit into a function since there is a similar instance above. It makes sense to do all this now since the intent of this patch is a code cleanup. 
This code was introduced previously, not in this patch. I'm only refactoring places I've noticed. And yes, there is a lot more places to refactor.
That is not just max you've mentioned. 
Currently LoopSize is defined as max (3, "estimated loop size"), BEInsns is just "2". So LoopSize is always greater than BEInsns.
However I agree, that moving UnrolledSize calculation into function and adding the assert to the function is a good point.
What I want to do is to move the whole threshold check to a function. And that requires modifications that would change current behavior - so that will go to the next patch.
You can look into previous comment for the discussion.


http://reviews.llvm.org/D19553