<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Apr 22, 2014 at 5:59 PM, <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank" class="cremed">hfinkel@anl.gov</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thanks for working on this!<br>
<br>
================<br>
Comment at: include/llvm/Analysis/LoopInfo.h:465<br>
@@ +464,3 @@<br>
<div class="">+<br>
+    // Try the pre-header first.<br>
+    if ((HeadBB = getLoopPreheader()) != nullptr) {<br>
</div>----------------<br>
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.<br>
<br>
Maybe looking in the preheader makes sense, but you should specifically look for the *last* location somehow I'd think.<br></blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
================<br>
Comment at: lib/Transforms/Utils/LoopUnroll.cpp:242<br>
@@ -234,1 +241,3 @@<br>
<div class="">+                               Twine("completely unrolled loop ") +<br>
+                                   Twine(TripCount) + " times");<br>
   } else {<br>
</div>----------------<br>
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 ;)<br>
<br>
How about: "completely unrolled loop with N iterations".<br></blockquote><div><br></div><div>D'oh! Fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<br>
================<br>
Comment at: lib/Transforms/Utils/LoopUnroll.cpp:246<br>
@@ -236,2 +245,3 @@<br>
<div class="">           << " by " << Count);<br>
+    Twine DiagMsg("unrolled loop " + Twine(Count) + " times ");<br>
     if (TripMultiple == 0 || BreakoutTrip != TripMultiple) {<br>
</div>----------------<br>
Here too, maybe something like: "unrolled loop by a factor of N"<br>
<br></blockquote><div><br></div><div>Fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
================<br>
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1194<br>
@@ +1193,3 @@<br>
<div class="">+        Twine("vectorized loop. Vectorization factor: ") + Twine(VF.Width) +<br>
+            ". Unroll factor: " + Twine(UF));<br>
+<br>
</div>----------------<br>
Sounds like we're calling this "interleaving factor". The capitalization seems odd here, nothing else is capitalized.<br>
<br></blockquote><div><br></div><div>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)'?</div><div></div></div>
<br></div><div class="gmail_extra"><br></div><div class="gmail_extra">Diego.</div></div>