[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

[Duncan Sands]

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.

thanks for explaining.  But will it actually work?  The GEP indices themselves
could already be a computation with nuw/nsw flags or be undef, which could
propagate through.  All kinds of transforms might already have munched on the
GEPs, exploiting "inbounds" to transform them to something "wrong" (from your
point of view).  Also, the name NoNUW could be better I reckon.  I mean: it's
not nuw wrap you really care about, you would also care about nsw or undef or
who knows what.  How about NoAssumptions (though I don't actually like this
either)?  In fact, since you have special needs I can't help feeling that you
should write your own routines and not make use of generic routines like this.

Ciao, Duncan.

>
> 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
>>> +}