[LLVMdev] [llvm-commits] [llvm] r157649 - /llvm/trunk/lib/Transforms/Scalar/BoundsChecking.cpp

[Hal Finkel]

On Wed, 30 May 2012 10:04:50 -0700
Andrew Trick <atrick at apple.com> wrote:

> Originally on llvm-commits.
> 
> On May 30, 2012, at 8:48 AM, Nuno Lopes <nunoplopes at sapo.pt> wrote:
> >> This is probably fine for now. It's a relatively safe use of
> >> SCEVExpander and the least effort approach, but generally I would
> >> like to encourage people to solve local rewrite problems with
> >> IRBuilder, not SCEVExpander, and build useful utilities for that
> >> purpose. Just because you use SCEV doesn't mean you have to use
> >> SCEVExpander. SCEVExpander may try to do more rewriting than you
> >> expect--it can create an indefinite number of new instructions. In
> >> general, it can create new phis, reassociate GEPs, drop "inbounds"
> >> and create ugly GEPS. Things I really hate to see before LSR! It
> >> also makes strong assumptions about valid input that the caller
> >> has to check first and is unable to bailout when it hits a strange
> >> case. Supporting SCEVExpander also imposes limitations on SCEV
> >> that would be nice to remove some day.

Andy,

Out of curiosity, could you be more specific? What limitations are we
talking about?

Thanks again,
Hal

> > 
> > Well, here I'm not rewriting anything /per se/. I'm generating a
> > whole new expression which represents the value of the index of a
> > GEP in the last loop iteration relative to the base object. In case
> > we are lucky, it will be just 'N-1', for example. Unless SCEV
> > cannot prove that the value doesn't overflow, and then we get a
> > complex expression (but nothing with PHIs, I guess).
> 
> And hopefully nothing with GEPs. So it's probably safe in your case.
> 
> >> As an alternative, you can find your GEP's underlying phi and grab
> >> the value from the preheader. SCEV will tell you the difference
> >> between getSCEVAtScope and the preheader's expression, hopefully
> >> in terms of a constant or value+constant, and you should be able
> >> to materialize an inbounds gep. If any of these steps fail to find
> >> a simple solution, you can easily bailout. I'm glossing over the
> >> issues, but it would be nice to have a utility like this outside
> >> or alongside current SCEVExpander. I think most of the code in
> >> SCEVExpander is good, I just don't think the current design is
> >> well suited for use outside LSR.
> > 
> > So you are suggesting, as an optimization, that if SCEV returns a
> > simple 'value+constant' solution, I can simply take the constant
> > and verify statically if the GEP will overflow or not. Adding
> > inbounds flag is not really in the scope of this pass. Isn't there
> > any pass that will do that?
> 
> You would have to do some pattern matching to make best use of
> existing values instead of the indiscriminate rewrite that
> SCEVExpander does. It's more work and maybe not worthwhile in your
> case. My response was mainly intended to warn other developers and
> have them go through the mental exercise: "what would I do if
> SCEVExpander didn't exist"... instead of instinctively using it as a
> general utility.
> 
> -Andy



-- 
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory