<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jun 24, 2014 at 10:43 PM, 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Agreed about preferring loop.vectorize to loop.vectorizer. Thanks for<br>
the reviews. Attached are the updated patches. Eli, can you submit<br>
these?<br>
<br></blockquote><div><br></div><div>It seems that test/Bitcode/upgrade-loop-metadata.ll.bc is missing:</div><div><br></div><div><div>test/Bitcode/Output/upgrade-loop-metadata.ll.script: line 1: /media/SSD1/llvm/llvm_svn_rw/test/Bitcode/upgrade-loop-metadata.ll.bc: No such file or directory</div>
<div>FileCheck error: '-' is empty.</div><div><br></div><div>--</div><div><br></div><div>********************</div><div>Testing Time: 74.68s</div><div>********************</div><div>Failing Tests (1):</div><div> LLVM :: Bitcode/upgrade-loop-metadata.ll</div>
</div><div><br></div><div><br></div><div><br></div><div>Eli</div><div><br></div><div> </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">
Thanks,<br>
Mark<br>
<div class=""><div class="h5"><br>
On Tue, Jun 24, 2014 at 6:40 PM, Tyler Nowicki <<a href="mailto:tnowicki@apple.com">tnowicki@apple.com</a>> wrote:<br>
> Hi Mark,<br>
><br>
> I also have a weak preference for the llvm.loop.unroll/vectorize option because I would like to add metadata for interleaving and I think interleave.enable would look better than interleaver.enable. Otherwise LGTM!<br>
><br>
> Tyler<br>
><br>
> On Jun 24, 2014, at 4:28 PM, Hal Finkel <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:<br>
><br>
>> ----- Original Message -----<br>
>>> From: "Mark Heffernan" <<a href="mailto:meheff@google.com">meheff@google.com</a>><br>
>>> To: "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>, "cfe-commits" <<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a>>, "Pekka Jääskeläinen"<br>
>>> <<a href="mailto:pekka.jaaskelainen@tut.fi">pekka.jaaskelainen@tut.fi</a>><br>
>>> Sent: Tuesday, June 24, 2014 4:18:30 PM<br>
>>> Subject: Re: [PATCH] Change loop unroll and vectorizer metadata prefix to 'llvm.loop.'<br>
>>><br>
>>> The attached patch includes a note in ReleaseNotes.rst and code to<br>
>>> autoupgrade llvm.vectorizer.* metadata strings to<br>
>>> llvm.loop.vectorizer.* when reading assembly and bitcode. PTAL.<br>
>><br>
>> Okay, thanks. That's better. However, I just noticed that the proposed mapping:<br>
>><br>
>>> llvm.vectorizer.* => llvm.loop.vectorizer.*<br>
>>> llvm.loopunroll.* => llvm.loop.unroll.*<br>
>><br>
>> is not grammatically consistent. I think that we should have ("llvm.loop.vectorize" and "llvm.loop.unroll") or ("llvm.loop.vectorizer" and "llvm.loop.unroller"). I have a weak preference for the first pair, but I care more about consistency than the choice.<br>
>><br>
>> -Hal<br>
>><br>
>>><br>
>>> Thanks!<br>
>>> Mark<br>
>>><br>
>>> On Tue, Jun 24, 2014 at 9:53 AM, Hal Finkel <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:<br>
>>>> ----- Original Message -----<br>
>>>>> From: "Mark Heffernan" <<a href="mailto:meheff@google.com">meheff@google.com</a>><br>
>>>>> To: "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>, "cfe-commits"<br>
>>>>> <<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a>>, "Pekka Jääskeläinen"<br>
>>>>> <<a href="mailto:pekka.jaaskelainen@tut.fi">pekka.jaaskelainen@tut.fi</a>><br>
>>>>> Sent: Tuesday, June 24, 2014 11:18:33 AM<br>
>>>>> Subject: Re: [PATCH] Change loop unroll and vectorizer metadata<br>
>>>>> prefix to 'llvm.loop.'<br>
>>>>><br>
>>>>> Thanks for the comments. I'll add something to the release notes.<br>
>>>>><br>
>>>>> On Tue, Jun 24, 2014 at 3:52 AM, Hal Finkel <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>><br>
>>>>> wrote:<br>
>>>>>> Moreover, for metadata that was supported by the 3.4 release,<br>
>>>>>> autoupgrade support should be added. Please do this before you<br>
>>>>>> commit.<br>
>>>>><br>
>>>>> By autoupgrade do you mean be permissive about what is accepted?<br>
>>>>> That<br>
>>>>> is, accept llvm.vectorizer.* and llvm.loop.vectorizer.* but only<br>
>>>>> emit<br>
>>>>> the llvm.loop form from the FE. Or do you mean try to rename the<br>
>>>>> metadata as soon as it parsed? I can see how to do the first case<br>
>>>>> (be<br>
>>>>> permissive) easily, but for the second (rename metadata) do you<br>
>>>>> have<br>
>>>>> any suggestions for the best place to cut in (I'm fairly new to<br>
>>>>> the<br>
>>>>> code base)?<br>
>>>><br>
>>>> No, I mean the second. I believe that most of the current logic for<br>
>>>> this is in lib/IR/AutoUpgrade.cpp<br>
>>>><br>
>>>> -Hal<br>
>>>><br>
>>>>><br>
>>>>> Thanks!<br>
>>>>> Mark<br>
>>>>><br>
>>>>>><br>
>>>>>> -Hal<br>
>>>>>><br>
>>>>>>><br>
>>>>>>> On Tue, Jun 24, 2014 at 12:30 AM, Mark Heffernan<br>
>>>>>>> <<a href="mailto:meheff@google.com">meheff@google.com</a>><br>
>>>>>>> wrote:<br>
>>>>>>>> These patches rename the loop unrolling and loop vectorizer<br>
>>>>>>>> metadata<br>
>>>>>>>> such that they have a common 'llvm.loop.' prefix. Metadata<br>
>>>>>>>> name<br>
>>>>>>>> changes:<br>
>>>>>>>><br>
>>>>>>>> llvm.vectorizer.* => llvm.loop.vectorizer.*<br>
>>>>>>>> llvm.loopunroll.* => llvm.loop.unroll.*<br>
>>>>>>>><br>
>>>>>>>> This was a suggestion from an earlier review<br>
>>>>>>>> (<a href="http://reviews.llvm.org/D4090" target="_blank">http://reviews.llvm.org/D4090</a>) which added the loop<br>
>>>>>>>> unrolling<br>
>>>>>>>> metadata. Two patches are attached. One for front-end and<br>
>>>>>>>> one<br>
>>>>>>>> for<br>
>>>>>>>> optimizer.<br>
>>>>>>>><br>
>>>>>>>> Mark<br>
>>>>>>><br>
>>>>>>><br>
>>>>>>><br>
>>>>>>> --<br>
>>>>>>> --PJ<br>
>>>>>>> _______________________________________________<br>
>>>>>>> cfe-commits mailing list<br>
>>>>>>> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>>>>>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
>>>>>>><br>
>>>>>><br>
>>>>>> --<br>
>>>>>> Hal Finkel<br>
>>>>>> Assistant Computational Scientist<br>
>>>>>> Leadership Computing Facility<br>
>>>>>> Argonne National Laboratory<br>
>>>>><br>
>>>><br>
>>>> --<br>
>>>> Hal Finkel<br>
>>>> Assistant Computational Scientist<br>
>>>> Leadership Computing Facility<br>
>>>> Argonne National Laboratory<br>
>>><br>
>><br>
>> --<br>
>> Hal Finkel<br>
>> Assistant Computational Scientist<br>
>> Leadership Computing Facility<br>
>> Argonne National Laboratory<br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
</div></div></blockquote></div><br></div></div>