[PATCH] Fix a performance problem in gep(gep ...) merging

[Xinliang David Li]

On Tue, Apr 14, 2015 at 10:58 AM, Hal Finkel <hfinkel at anl.gov> wrote:

> ----- Original Message -----
> > From: "Xinliang David Li" <xinliangli at gmail.com>
> > To: "Nick Lewycky" <nicholas at mxc.ca>
> > Cc: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>,
> > reviews+D8911+public+8f2845face98911b at reviews.llvm.org
> > Sent: Tuesday, April 14, 2015 10:23:36 AM
> > Subject: Re: [PATCH] Fix a performance problem in gep(gep ...) merging
> >
> >
> >
> > IIUC, GEP merging can be harmful regardless of where the target is in
> > loop context or not. Wei's heuristic filters out lots of harmful
> > cases. A more general fix might need introduce more sophisticated
> > cost analysis -- it is possible (as mentioned by Quentin) for this
> > simple heuristic to filter out beneficial merging, but test cases
> > are needed for that.
>
> One thing we need to keep in mind here is that the reduction in GEP depth
> directly affects the precision of BasicAA (and other code that calls
> GetUnderlyingObject/GetUnderlyingObjects. These things have small recursion
> cutoffs (6 currently). computeKnownBits, which looks through GEPs to find
> pointer alignments is another example. The aggressive merging strategy
> supports these small cutoffs; if we reduce the amount of merging that we
> do, we may need to re-tune these cutoffs.
>

In practice, how often do we see long chains of GEPs? 6 seems reasonably
long. The alias analysis does not really depend on the merged form of GEP
to be more precise (other than the arbitrary limitation imposed on long GEP
chains).

In fact, there are cases where merged GEP actually confuses aliaser.
Consider the following case:

extern int aa[];

int foo(int k, int n
#ifdef NOMERGE
        , int *bb
#endif
        )
{
   int i, s;
   s = 0;
#ifndef NOMERGE
   int *bb = &aa[k];
#endif
   s = bb[n];
   bb[n+1] = 20;
   s+=bb[n];

   return s;
}

When built with -DNOMERGE or without but with instcombine disabled, GVN is
able to remove the second load. However, with instcombine turned on and GEP
merged, the load remains after the store.

David





>
>  -Hal
>
> >
> >
> > David
> >
> >
> > On Tue, Apr 14, 2015 at 1:16 AM, Nick Lewycky < nicholas at mxc.ca >
> > wrote:
> >
> >
> > Quentin Colombet wrote:
> >
> >
> > The thing with the "both constants" approach is that we may miss some
> > cases where an addressing mode may apply. In particular, on x86,
> > when one argument of the add is a constant, we would be able to
> > match reg1 + reg2 + displacement.
> >
> > That being said, your numbers imply this is not a problem in
> > practice, and most targets do not support reg1 + reg2 + reg3/imm
> > addressing mode. Anyway, we could recover from that later on if
> > needed.
> >
> > So LGTM.
> >
> > I might be misunderstanding why this patch helps, please bear with
> > me. Is this not a special case of a general problem, mainly that
> > we're merging something into a loop that was previously two
> > operations one inside and one outside the loop? Why wouldn't this
> > problem come up in other ways? We already have other problems where
> > we introduce loop carried dependencies where ones previously didn't
> > exist, shouldn't we approach this by adding an API to detect those
> > and then making transforms conditional on that?
> >
> > Nick
> >
> >
> >
> >
> >
> >
> > Thanks Wei!
> > -Quentin
> >
> >
> > REPOSITORY
> > rL LLVM
> >
> > http://reviews.llvm.org/D8911
> >
> > EMAIL PREFERENCES
> > http://reviews.llvm.org/ settings/panel/ emailpreferences/
> >
> >
> >
> >
> > ______________________________ _________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/ mailman/listinfo/llvm-commits
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150415/7b73acf9/attachment.html>