[PATCH] Change loop unroll and vectorizer metadata prefix to 'llvm.loop.'

[Mark Heffernan]

Agreed about preferring loop.vectorize to loop.vectorizer.  Thanks for
the reviews.  Attached are the updated patches.  Eli, can you submit
these?

Thanks,
Mark

On Tue, Jun 24, 2014 at 6:40 PM, Tyler Nowicki <tnowicki at apple.com> wrote:
> Hi Mark,
>
> 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!
>
> Tyler
>
> On Jun 24, 2014, at 4:28 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>
>> ----- Original Message -----
>>> From: "Mark Heffernan" <meheff at google.com>
>>> To: "Hal Finkel" <hfinkel at anl.gov>
>>> Cc: llvm-commits at cs.uiuc.edu, "cfe-commits" <cfe-commits at cs.uiuc.edu>, "Pekka Jääskeläinen"
>>> <pekka.jaaskelainen at tut.fi>
>>> Sent: Tuesday, June 24, 2014 4:18:30 PM
>>> Subject: Re: [PATCH] Change loop unroll and vectorizer metadata prefix to 'llvm.loop.'
>>>
>>> The attached patch includes a note in ReleaseNotes.rst and code to
>>> autoupgrade llvm.vectorizer.* metadata strings to
>>> llvm.loop.vectorizer.* when reading assembly and bitcode.  PTAL.
>>
>> Okay, thanks. That's better. However, I just noticed that the proposed mapping:
>>
>>> llvm.vectorizer.* => llvm.loop.vectorizer.*
>>> llvm.loopunroll.* => llvm.loop.unroll.*
>>
>> 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.
>>
>> -Hal
>>
>>>
>>> Thanks!
>>> Mark
>>>
>>> On Tue, Jun 24, 2014 at 9:53 AM, Hal Finkel <hfinkel at anl.gov> wrote:
>>>> ----- Original Message -----
>>>>> From: "Mark Heffernan" <meheff at google.com>
>>>>> To: "Hal Finkel" <hfinkel at anl.gov>
>>>>> Cc: llvm-commits at cs.uiuc.edu, "cfe-commits"
>>>>> <cfe-commits at cs.uiuc.edu>, "Pekka Jääskeläinen"
>>>>> <pekka.jaaskelainen at tut.fi>
>>>>> Sent: Tuesday, June 24, 2014 11:18:33 AM
>>>>> Subject: Re: [PATCH] Change loop unroll and vectorizer metadata
>>>>> prefix to 'llvm.loop.'
>>>>>
>>>>> Thanks for the comments.  I'll add something to the release notes.
>>>>>
>>>>> On Tue, Jun 24, 2014 at 3:52 AM, Hal Finkel <hfinkel at anl.gov>
>>>>> wrote:
>>>>>> Moreover, for metadata that was supported by the 3.4 release,
>>>>>> autoupgrade support should be added. Please do this before you
>>>>>> commit.
>>>>>
>>>>> By autoupgrade do you mean be permissive about what is accepted?
>>>>> That
>>>>> is, accept llvm.vectorizer.* and llvm.loop.vectorizer.* but only
>>>>> emit
>>>>> the llvm.loop form from the FE.  Or do you mean try to rename the
>>>>> metadata as soon as it parsed?  I can see how to do the first case
>>>>> (be
>>>>> permissive) easily, but for the second (rename metadata) do you
>>>>> have
>>>>> any suggestions for the best place to cut in (I'm fairly new to
>>>>> the
>>>>> code base)?
>>>>
>>>> No, I mean the second. I believe that most of the current logic for
>>>> this is in lib/IR/AutoUpgrade.cpp
>>>>
>>>> -Hal
>>>>
>>>>>
>>>>> Thanks!
>>>>> Mark
>>>>>
>>>>>>
>>>>>> -Hal
>>>>>>
>>>>>>>
>>>>>>> On Tue, Jun 24, 2014 at 12:30 AM, Mark Heffernan
>>>>>>> <meheff at google.com>
>>>>>>> wrote:
>>>>>>>> These patches rename the loop unrolling and loop vectorizer
>>>>>>>> metadata
>>>>>>>> such that they have a common 'llvm.loop.' prefix.  Metadata
>>>>>>>> name
>>>>>>>> changes:
>>>>>>>>
>>>>>>>> llvm.vectorizer.* => llvm.loop.vectorizer.*
>>>>>>>> llvm.loopunroll.* => llvm.loop.unroll.*
>>>>>>>>
>>>>>>>> This was a suggestion from an earlier review
>>>>>>>> (http://reviews.llvm.org/D4090) which added the loop
>>>>>>>> unrolling
>>>>>>>> metadata.  Two patches are attached.  One for front-end and
>>>>>>>> one
>>>>>>>> for
>>>>>>>> optimizer.
>>>>>>>>
>>>>>>>> Mark
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> --PJ
>>>>>>> _______________________________________________
>>>>>>> cfe-commits mailing list
>>>>>>> cfe-commits at cs.uiuc.edu
>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> Hal Finkel
>>>>>> Assistant Computational Scientist
>>>>>> Leadership Computing Facility
>>>>>> Argonne National Laboratory
>>>>>
>>>>
>>>> --
>>>> Hal Finkel
>>>> Assistant Computational Scientist
>>>> Leadership Computing Facility
>>>> Argonne National Laboratory
>>>
>>
>> --
>> Hal Finkel
>> Assistant Computational Scientist
>> Leadership Computing Facility
>> Argonne National Laboratory
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rename.clang.patch
Type: text/x-patch
Size: 8221 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140624/dc45ffac/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rename.llvm.patch
Type: text/x-patch
Size: 22213 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140624/dc45ffac/attachment-0001.bin>