<div dir="ltr">Is this patch good to go?  The changes we talked about (renaming metadata) will be in follow up patches.<div><br></div><div>Mark</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jul 18, 2014 at 10:28 AM, Mark Heffernan <span dir="ltr"><<a href="mailto:meheff@google.com" target="_blank">meheff@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">================<br>
Comment at: docs/LangRef.rst:2965<br>
@@ +2964,3 @@<br>
+bit operand value is 0 loop unrolling is disabled. A value of 1<br>
+indicates that the loop should be fully unrolled. For example:<br>
+<br>
----------------<br>
</div><div class="">Mark Heffernan wrote:<br>
> <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> wrote:<br>
> > I find this confusing. Specifically, to get partial unrolling, what needs to happen? If I want partial unrolling with a specific count, do I need both the 'count' and 'enable' metadata?<br>
> I added a bit more explanation about that case.<br>
><br>
> I'd imagine an underlying cause of confusion is that ``llvm.loop.unroll.enable 1`` doesn't exactly mean enable.  It means try to fully unroll the loop.  So we want the following possible hints:<br>
><br>
> don't unroll<br>
> unroll fully<br>
> unroll by N<br>
><br>
> This doesn't exactly map nicely to the two metadata we have (one with boolean operand, one with i32).  Maybe a cleaner way to have done this is with the following metadata:<br>
><br>
> llvm.loop.unroll.disable (no operand)<br>
> llvm.loop.unroll.fully_unroll (no operand)<br>
> llvm.loop.unroll.count i32<br>
><br>
> And only allow a single loop unroll metadata node per loop.  Worth making this change?<br>
</div>I should add that this confusion is also baked into the pragma syntax.  The following means unroll fully:<br>
<br>
#pragma clang loop unroll(enable)<br>
<br>
<a href="http://reviews.llvm.org/D4576" target="_blank">http://reviews.llvm.org/D4576</a><br>
<br>
<br>
</blockquote></div><br></div>