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
