[PATCH] Add optimization remarks to the loop unroller and vectorizer.

[Diego Novillo]

On Tue, Apr 22, 2014 at 5:59 PM, hfinkel at anl.gov <hfinkel at anl.gov> wrote:

> Thanks for working on this!
>
> ================
> Comment at: include/llvm/Analysis/LoopInfo.h:465
> @@ +464,3 @@
> +
> +    // Try the pre-header first.
> +    if ((HeadBB = getLoopPreheader()) != nullptr) {
> ----------------
> Why are you looking in the pre-header first? That does not seem to make
> sense because the start of the preheader could be pretty far away from the
> start of the loop.
>
> Maybe looking in the preheader makes sense, but you should specifically
> look for the *last* location somehow I'd think.
>

It was a toss up. For single-bb loops, the header is the body, which means
that the diagnostic shows up at the body of the loop.  I did not find cases
where the preheader had its first instruction far from the lexical start of
the block, but maybe I wasn't looking hard enough.


>
> ================
> Comment at: lib/Transforms/Utils/LoopUnroll.cpp:242
> @@ -234,1 +241,3 @@
> +                               Twine("completely unrolled loop ") +
> +                                   Twine(TripCount) + " times");
>    } else {
> ----------------
> This message should be improved. If I unroll a loop with 5 iterations,
> I've not unrolled it 5 times -- then I'd have 25 iterations ;)
>
> How about: "completely unrolled loop with N iterations".
>

D'oh! Fixed.


>
> ================
> Comment at: lib/Transforms/Utils/LoopUnroll.cpp:246
> @@ -236,2 +245,3 @@
>            << " by " << Count);
> +    Twine DiagMsg("unrolled loop " + Twine(Count) + " times ");
>      if (TripMultiple == 0 || BreakoutTrip != TripMultiple) {
> ----------------
> Here too, maybe something like: "unrolled loop by a factor of N"
>
>
Fixed.


> ================
> Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1194
> @@ +1193,3 @@
> +        Twine("vectorized loop. Vectorization factor: ") +
> Twine(VF.Width) +
> +            ". Unroll factor: " + Twine(UF));
> +
> ----------------
> Sounds like we're calling this "interleaving factor". The capitalization
> seems odd here, nothing else is capitalized.
>
>
Well, the punctuation kind of forced me to use capitals. Perhaps if I put
the values in side braces '(vectorization factor: N, interleaving factor:
M)'?


Diego.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140422/e7be6855/attachment.html>