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

[Andrew Trick]

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>