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

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.
> 
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120530/c91bd957/attachment.html>

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

On May 31, 2012, at 9:55 AM, Hal Finkel <hfinkel at anl.gov> wrote:
>>>> 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?
The one I remember now is the inability to analyze inttoptr/ptrtoint. There are
probably more though.

-Andy
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120531/4600d815/attachment.html>