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

Nuno Lopes nunoplopes at sapo.pt
Wed May 30 08:48:43 PDT 2012


>> -  if ((GEP = dyn_cast<GEPOperator>(Ptr))) {
>> +  // try to hoist the check if the instruction is inside a loop
>> +  Value *LoopOffset = 0;
>> +  if (Loop *L = LI->getLoopFor(Builder->GetInsertPoint()->getParent())) {
>> +    const SCEV *PtrSCEV  = SE->getSCEVAtScope(Ptr, L->getParentLoop());
>> +    const SCEV *BaseSCEV = SE->getPointerBase(PtrSCEV);
>> +
>> +    if (const SCEVUnknown *PointerBase = dyn_cast<SCEVUnknown>(BaseSCEV)) {
>> +      Ptr = PointerBase->getValue()->stripPointerCasts();
>> +      Instruction *InsertPoint =  
>> L->getLoopPreheader()->getFirstInsertionPt();
>> +      Builder->SetInsertPoint(InsertPoint);
>> +
>> +      SCEVExpander Expander(*SE, "bounds-checking");
>> +      const SCEV *OffsetSCEV = SE->getMinusSCEV(PtrSCEV, PointerBase);
>> +      LoopOffset = Expander.expandCodeFor(OffsetSCEV, SizeTy, InsertPoint);
>> +    }
>> +  }
>> +
>> +  GEPOperator *GEP = dyn_cast<GEPOperator>(Ptr);
>> +  if (GEP) {
>
> 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).


> 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?

Nuno



More information about the llvm-commits mailing list