[llvm-commits] [llvm] r156585 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombine.h lib/Transforms/InstCombine/InstCombineAddSub.cpp lib/Transforms/InstCombine/InstCombineCalls.cpp test/Transforms/InstCombine/objsize.ll

Nuno Lopes nunoplopes at sapo.pt
Mon May 14 08:40:38 PDT 2012


> Hi Nuno,
>
>> objectsize: add support for GEPs with non-constant indexes
>> add an additional parameter to InstCombiner::EmitGEPOffset() to  
>> force it to *not* emit operations with NUW flag
>
> why?  It seems strange - can you please explain.

Sure!
The idea is that even if a GEP has the inbounds flag, we want to have  
well-defined semantics for the computation of the offset. Otherwise  
the bounds check could be eliminated because of the undefinedness of  
NUW on overflow.
Basically this parameter makes EmitGEPOffset ignore the inbounds attribute.

Nuno


> Ciao, Duncan.
>
>> Modified:
>>      llvm/trunk/lib/Transforms/InstCombine/InstCombine.h
>>      llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
>>      llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
>>      llvm/trunk/test/Transforms/InstCombine/objsize.ll
>>
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombine.h
>> URL:  
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombine.h?rev=156585&r1=156584&r2=156585&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombine.h (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombine.h Thu May 10  
>> 18:17:35 2012
>> @@ -226,7 +226,7 @@
>>                                    bool DoXform = true);
>>     Instruction *transformSExtICmp(ICmpInst *ICI, Instruction&CI);
>>     bool WillNotOverflowSignedAdd(Value *LHS, Value *RHS);
>> -  Value *EmitGEPOffset(User *GEP);
>> +  Value *EmitGEPOffset(User *GEP, bool NoNUW = false);
>>
>>   public:
>>     // InsertNewInstBefore - insert an instruction New before  
>> instruction Old
>>
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp
>> URL:  
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp?rev=156585&r1=156584&r2=156585&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAddSub.cpp Thu  
>> May 10 18:17:35 2012
>> @@ -423,7 +423,8 @@
>>   /// EmitGEPOffset - Given a getelementptr  
>> instruction/constantexpr, emit the
>>   /// code necessary to compute the offset from the base pointer  
>> (without adding
>>   /// in the base pointer).  Return the result as a signed integer  
>> of intptr size.
>> -Value *InstCombiner::EmitGEPOffset(User *GEP) {
>> +/// If NoNUW is true, then the NUW flag is not used.
>> +Value *InstCombiner::EmitGEPOffset(User *GEP, bool NoNUW) {
>>     TargetData&TD = *getTargetData();
>>     gep_type_iterator GTI = gep_type_begin(GEP);
>>     Type *IntPtrTy = TD.getIntPtrType(GEP->getContext());
>> @@ -431,7 +432,7 @@
>>
>>     // If the GEP is inbounds, we know that none of the addressing  
>> operations will
>>     // overflow in an unsigned sense.
>> -  bool isInBounds = cast<GEPOperator>(GEP)->isInBounds();
>> +  bool isInBounds = cast<GEPOperator>(GEP)->isInBounds()&&  !NoNUW;
>>
>>     // Build a mask for high order bits.
>>     unsigned IntPtrWidth = TD.getPointerSizeInBits();
>>
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
>> URL:  
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp?rev=156585&r1=156584&r2=156585&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp Thu  
>> May 10 18:17:35 2012
>> @@ -173,7 +173,7 @@
>>                               uint64_t Penalty, TargetData *TD,
>>                               InstCombiner::BuilderTy *Builder) {
>>     if (GlobalVariable *GV = dyn_cast<GlobalVariable>(Alloc)) {
>> -    if (GV->hasUniqueInitializer()) {
>> +    if (GV->hasDefinitiveInitializer()) {
>>         Constant *C = GV->getInitializer();
>>         Size = TD->getTypeAllocSize(C->getType());
>>         return 1;
>> @@ -198,7 +198,8 @@
>>       if (Penalty<  2)
>>         return 2;
>>
>> -    SizeValue = Builder->CreateMul(Builder->getInt64(Size), ArraySize);
>> +    SizeValue = ConstantInt::get(ArraySize->getType(), Size);
>> +    SizeValue = Builder->CreateMul(SizeValue, ArraySize);
>>       return 0;
>>
>>     } else if (CallInst *MI = extractMallocCall(Alloc)) {
>> @@ -320,22 +321,12 @@
>>
>>       // Get to the real allocated thing and offset as fast as possible.
>>       Value *Op1 = II->getArgOperand(0)->stripPointerCasts();
>> +    GEPOperator *GEP;
>>
>> -    uint64_t Offset = 0;
>> -    Value *OffsetValue;
>> -    bool ConstOffset = true;
>> -
>> -    // Try to look through constant GEPs.
>> -    if (GEPOperator *GEP = dyn_cast<GEPOperator>(Op1)) {
>> -      if (!GEP->hasAllConstantIndices()) return 0;
>> -
>> -      // Get the current byte offset into the thing. Use the original
>> -      // operand in case we're looking through a bitcast.
>> -      SmallVector<Value*, 8>  Ops(GEP->idx_begin(), GEP->idx_end());
>> -      if (!GEP->getPointerOperandType()->isPointerTy())
>> +    if ((GEP = dyn_cast<GEPOperator>(Op1))) {
>> +      // check if we will be able to get the offset
>> +      if (!GEP->hasAllConstantIndices()&&  Penalty<  2)
>>           return 0;
>> -      Offset = TD->getIndexedOffset(GEP->getPointerOperandType(), Ops);
>> -
>>         Op1 = GEP->getPointerOperand()->stripPointerCasts();
>>       }
>>
>> @@ -349,28 +340,36 @@
>>       if (ConstAlloc == 2)
>>         return 0;
>>
>> -    if (ConstOffset&&  ConstAlloc) {
>> +    uint64_t Offset = 0;
>> +    Value *OffsetValue = 0;
>> +
>> +    if (GEP) {
>> +      if (GEP->hasAllConstantIndices()) {
>> +        SmallVector<Value*, 8>  Ops(GEP->idx_begin(), GEP->idx_end());
>> +        assert(GEP->getPointerOperandType()->isPointerTy());
>> +        Offset = TD->getIndexedOffset(GEP->getPointerOperandType(), Ops);
>> +      } else
>> +        OffsetValue = EmitGEPOffset(GEP, true /*NoNUW*/);
>> +    }
>> +
>> +    if (!OffsetValue&&  ConstAlloc) {
>>         if (Size<  Offset) {
>>           // Out of bounds
>>           return ReplaceInstUsesWith(CI, ConstantInt::get(ReturnTy, 0));
>>         }
>>         return ReplaceInstUsesWith(CI, ConstantInt::get(ReturnTy,  
>> Size-Offset));
>> +    }
>>
>> -    } else if (Penalty>= 2) {
>> -      if (ConstOffset)
>> -        OffsetValue = Builder->getInt64(Offset);
>> -      if (ConstAlloc)
>> -        SizeValue = Builder->getInt64(Size);
>> -
>> -      Value *Val = Builder->CreateSub(SizeValue, OffsetValue);
>> -      Val = Builder->CreateTrunc(Val, ReturnTy);
>> -      // return 0 if there's an overflow
>> -      Value *Cmp = Builder->CreateICmpULT(SizeValue, OffsetValue);
>> -      Val = Builder->CreateSelect(Cmp, ConstantInt::get(ReturnTy, 0), Val);
>> -      return ReplaceInstUsesWith(CI, Val);
>> -
>> -    } else
>> -      return 0;
>> +    if (!OffsetValue)
>> +      OffsetValue = ConstantInt::get(ReturnTy, Offset);
>> +    if (ConstAlloc)
>> +      SizeValue = ConstantInt::get(ReturnTy, Size);
>> +
>> +    Value *Val = Builder->CreateSub(SizeValue, OffsetValue);
>> +    // return 0 if there's an overflow
>> +    Value *Cmp = Builder->CreateICmpULT(SizeValue, OffsetValue);
>> +    Val = Builder->CreateSelect(Cmp, ConstantInt::get(ReturnTy, 0), Val);
>> +    return ReplaceInstUsesWith(CI, Val);
>>     }
>>     case Intrinsic::bswap:
>>       // bswap(bswap(x)) ->  x
>>
>> Modified: llvm/trunk/test/Transforms/InstCombine/objsize.ll
>> URL:  
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/objsize.ll?rev=156585&r1=156584&r2=156585&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/InstCombine/objsize.ll (original)
>> +++ llvm/trunk/test/Transforms/InstCombine/objsize.ll Thu May 10  
>> 18:17:35 2012
>> @@ -168,3 +168,28 @@
>>   ; CHECK-NEXT: ret i32 30
>>     ret i32 %objsize
>>   }
>> +
>> +; CHECK: @test9
>> +define i32 @test9(i32 %x, i32 %y) nounwind {
>> +  %a = alloca [3 x [4 x double]], align 8
>> +  %1 = getelementptr inbounds [3 x [4 x double]]* %a, i32 0, i32 %x
>> +  %2 = getelementptr inbounds [4 x double]* %1, i32 0, i32 %y
>> +  %3 = bitcast double* %2 to i8*
>> +  %objsize = call i32 @llvm.objectsize.i32(i8* %3, i1 false, i32 2)
>> +  ret i32 %objsize
>> +; CHECK-NEXT: shl i32 %x, 5
>> +; CHECK-NEXT: shl i32 %y, 3
>> +; CHECK-NEXT: add i32
>> +; CHECK-NEXT: sub i32 96,
>> +; CHECK-NEXT: icmp ugt i32 {{.*}}, 96
>> +; CHECK-NEXT: select i1 {{.*}}, i32 0,
>> +}
>> +
>> +; CHECK: @overflow
>> +define i32 @overflow() {
>> +  %alloc = call noalias i8* @malloc(i32 21) nounwind
>> +  %gep = getelementptr inbounds i8* %alloc, i32 50
>> +  %objsize = call i32 @llvm.objectsize.i32(i8* %gep, i1 false, i32  
>> 0) nounwind readonly
>> +; CHECK-NEXT: ret i32 0
>> +  ret i32 %objsize
>> +}



More information about the llvm-commits mailing list