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

Hal Finkel hfinkel at anl.gov
Tue Jun 24 16:28:26 PDT 2014


----- 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




More information about the cfe-commits mailing list