<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jul 18, 2014 at 12:01 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Yes, I think so.<br></blockquote><div><br></div><div>Thanks.  Committed as  r213397.</div><div><br></div><div>Mark</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<br>
 -Hal<br>
<div class="im"><br>
----- Original Message -----<br>
> From: "Mark Heffernan" <<a href="mailto:meheff@google.com">meheff@google.com</a>><br>
</div><div class="im">> To: "Mark Heffernan" <<a href="mailto:meheff@google.com">meheff@google.com</a>>, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>><br>
> Cc: <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
</div><div class="im">> Sent: Friday, July 18, 2014 1:08:56 PM<br>
> Subject: Re: [PATCH] Add loop unrolling metadata descriptions to LangRef.rst<br>
><br>
><br>
</div><div class=""><div class="h5">> Is this patch good to go? The changes we talked about (renaming<br>
> metadata) will be in follow up patches.<br>
><br>
><br>
> Mark<br>
><br>
><br>
><br>
> On Fri, Jul 18, 2014 at 10:28 AM, Mark Heffernan < <a href="mailto:meheff@google.com">meheff@google.com</a><br>
> > wrote:<br>
><br>
><br>
><br>
> ================<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>
><br>
> 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,<br>
> > > what needs to happen? If I want partial unrolling with a<br>
> > > specific count, do I need both the 'count' and 'enable'<br>
> > > metadata?<br>
> > I added a bit more explanation about that case.<br>
> ><br>
> > I'd imagine an underlying cause of confusion is that<br>
> > ``llvm.loop.unroll.enable 1`` doesn't exactly mean enable. It<br>
> > means try to fully unroll the loop. So we want the following<br>
> > 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<br>
> > with boolean operand, one with i32). Maybe a cleaner way to have<br>
> > 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<br>
> > making this change?<br>
> I should add that this confusion is also baked into the pragma<br>
> 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>
><br>
><br>
<br>
</div></div><div class=""><div class="h5">--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</div></div></blockquote></div><br></div></div>