[PATCH] CSE on GEP indices

[Eli Bendersky]

On Thu, Apr 24, 2014 at 11:28 AM, Hal Finkel <hfinkel at anl.gov> wrote:

> ----- Original Message -----
> > From: "Eli Bendersky" <eliben at google.com>
> > To: "Hal Finkel" <hfinkel at anl.gov>
> > Cc: reviews+D3462+public+cbb08f1e0c1f69e1 at reviews.llvm.org,
> llvm-commits at cs.uiuc.edu, "Jingyue Wu"
> > <jingyue at google.com>, "justin holewinski" <justin.holewinski at gmail.com>
> > Sent: Thursday, April 24, 2014 1:23:34 PM
> > Subject: Re: [PATCH] CSE on GEP indices
> >
> >
> >
> >
> >
> >
> >
> > On Thu, Apr 24, 2014 at 11:13 AM, Hal Finkel < hfinkel at anl.gov >
> > wrote:
> >
> >
> >
> > ----- Original Message -----
> > > From: "Eli Bendersky" < eliben at google.com >
> > > To: jingyue at google.com , hfinkel at anl.gov , eliben at google.com ,
> > > "justin holewinski" < justin.holewinski at gmail.com >
> > > Cc: llvm-commits at cs.uiuc.edu
> > > Sent: Thursday, April 24, 2014 1:06:05 PM
> > > Subject: Re: [PATCH] CSE on GEP indices
> > >
> > > ================
> > > Comment at: lib/Target/NVPTX/NVPTXTargetMachine.cpp:150
> > > @@ -149,5 +149,3 @@
> > > addPass(createNVPTXFavorNonGenericAddrSpacesPass());
> > > - // The FavorNonGenericAddrSpaces pass may remove instructions and
> > > leave some
> > > - // values unused. Therefore, we run a DCE pass right afterwards.
> > > We could
> > > - // remove unused values in an ad-hoc manner, but it requires
> > > manual work and
> > > - // might be error-prone.
> > > + addPass(createSeparateConstOffsetFromGEPPass());
> > > + // The SeparateConstOffsetFromGEP pass creates variadic bases
> > > that
> > > can be used
> > > ----------------
> > > Since this pass is not target independent, I'm not sure this is the
> > > right place to put it. It makes more sense to add it as part of a
> > > custom compiler optimization pipeline, or maybe even in opt itself?
> >
> > Why do you say this? As I see it, this pass is essentially LSR for
> > the unrolled portion of multidimensional loop nests. And I asked
> > them to add the TTI checks and move it because it is target
> > independent. Because of the complex addressing modes available on
> > x86, I'm not sure it will help there, but I suspect it will prove
> > useful for several other targets (PPC, for example).
> >
> > However, like LSR, it is not a canonicalization transformation, so it
> > belongs, along with LSR, as a backend IR pass.
> >
> >
> >
> > Fair enough. I just wanted to avoid code duplication among different
> > passes, because NVPTX has its own addIRPasses. Another backend
> > wanting to use this (PPC, for example) would need to duplicate this
> > logic. Maybe it belongs in TargetPassConfig::addIRPasses() then ?
>
> I'm not sure. We have some passes that interested targets are expected to
> add on their own (if conversion, for example). I don't have a strong
> opinion here, but I'm inclined to let the targets add the passes for now
> and then later, if we see a fairly-universal ordering, we can add things
> into TargetPassConfig with the necessary callbacks.


OK, that sounds reasonable to me.

Eli
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140424/c836315e/attachment.html>