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

[Tyler Nowicki]

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