<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jul 18, 2014 at 10:28 AM, 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">


<div><div>> This doesn't exactly map nicely to the two metadata we have (one with<br></div></div><div><div>
> boolean operand, one with i32).  Maybe a cleaner way to have done<br>
> 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>
<br>
</div></div>Yes, I think so. Maybe we can even do it before we branch for the release :-)<br></blockquote><div><br></div><div>Started working on this.  If it makes sense to change the IR metadata to more closely match the intended meaning, then I suppose it makes sense to change the pragma notation as well.  Maybe something like:</div>

<div><br></div><div>#pragam clang loop unroll(disable)<br></div><div>#pragma clang loop unroll(full)</div><div>#pragma clang loop unroll_count(value)</div><div><br></div><div>The difference here is that previously "#pragma clang loop unroll(enable)" meant full unrolling, now "unroll(full)" will means that.  Also, a loop now could now only have a single loop unrolling pragma (previously a loop could have unroll(enable) and unroll_count(N) pragmas).</div>
<div><br></div><div>Overall I think this makes the unroll pragma clearer (no more 'enable' magically meaning full unroll).  I'll send out the patch shortly for review.</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>
<br>
><br>
> <a href="http://reviews.llvm.org/D4576" target="_blank">http://reviews.llvm.org/D4576</a><br>
><br>
><br>
><br>
<span><font color="#888888"><br>
--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span></blockquote></div><br></div></div>