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

[Eli Bendersky]

On Tue, Jun 24, 2014 at 10:43 PM, Mark Heffernan <meheff at google.com> wrote:

> Agreed about preferring loop.vectorize to loop.vectorizer.  Thanks for
> the reviews.  Attached are the updated patches.  Eli, can you submit
> these?
>
>
It seems that test/Bitcode/upgrade-loop-metadata.ll.bc is missing:

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
FileCheck error: '-' is empty.

--

********************
Testing Time: 74.68s
********************
Failing Tests (1):
    LLVM :: Bitcode/upgrade-loop-metadata.ll



Eli



> 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 --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140625/c1129473/attachment.html>