On Tue, Jun 24, 2014 at 10:43 PM, Mark Heffernan <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote:
> >
> >> ----- Original Message -----
> >>> From: "Mark Heffernan" <[email protected]>
> >>> To: "Hal Finkel" <[email protected]>
> >>> Cc: [email protected], "cfe-commits" <[email protected]>,
> "Pekka Jääskeläinen"
> >>> <[email protected]>
> >>> 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 <[email protected]> wrote:
> >>>> ----- Original Message -----
> >>>>> From: "Mark Heffernan" <[email protected]>
> >>>>> To: "Hal Finkel" <[email protected]>
> >>>>> Cc: [email protected], "cfe-commits"
> >>>>> <[email protected]>, "Pekka Jääskeläinen"
> >>>>> <[email protected]>
> >>>>> 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 <[email protected]>
> >>>>> 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
> >>>>>>> <[email protected]>
> >>>>>>> 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
> >>>>>>> [email protected]
> >>>>>>> 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
> >> [email protected]
> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
