[llvm] r233116 - Remove an InstCombine that seems to have become redundant.

[Rafael Espíndola]

Looks like this was

https://llvm.org/bugs/show_bug.cgi?id=23010

On 24 March 2015 at 17:53, David Blaikie <dblaikie at gmail.com> wrote:
> Fires in compiler-rt, according to the bots, so I've reverted in r233121
>
> On Tue, Mar 24, 2015 at 2:31 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> Author: dblaikie
>> Date: Tue Mar 24 16:31:31 2015
>> New Revision: 233116
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=233116&view=rev
>> Log:
>> Remove an InstCombine that seems to have become redundant.
>>
>> Assert that this doesn't fire - I'll remove all of this later, but just
>> leaving it in for a while in case this is firing & we just don't have
>> test coverage.
>>
>> Modified:
>>     llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
>>     llvm/trunk/test/Transforms/InstCombine/2007-05-14-Crash.ll
>>
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp?rev=233116&r1=233115&r2=233116&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp (original)
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp Tue Mar 24
>> 16:31:31 2015
>> @@ -1455,35 +1455,20 @@ Instruction *InstCombiner::commonPointer
>>      // GEP computes a constant offset, see if we can convert these three
>>      // instructions into fewer.  This typically happens with unions and
>> other
>>      // non-type-safe code.
>> -    unsigned AS = GEP->getPointerAddressSpace();
>> -    unsigned OffsetBits = DL.getPointerSizeInBits(AS);
>> -    APInt Offset(OffsetBits, 0);
>> +    // Looks like this never actually fires due to bitcast+gep folding
>> happening
>> +    // in InstCombiner::visitGetElementPtrInst where the bitcast operand
>> to a
>> +    // gep is folded into the gep if possible, before we consider whether
>> that
>> +    // gep is used in a bitcast as well.
>> +    // Let's assert that this wouldn't fire just to be sure.
>> +#ifndef NDEBUG
>> +    APInt Offset(DL.getPointerSizeInBits(GEP->getPointerAddressSpace()),
>> 0);
>>      BitCastInst *BCI = dyn_cast<BitCastInst>(GEP->getOperand(0));
>> -    if (GEP->hasOneUse() && BCI && GEP->accumulateConstantOffset(DL,
>> Offset)) {
>> -      // FIXME: This is insufficiently tested - just a no-crash test
>> -      // (test/Transforms/InstCombine/2007-05-14-Crash.ll)
>> -      //
>> -      // Get the base pointer input of the bitcast, and the type it
>> points to.
>> -      Value *OrigBase = BCI->getOperand(0);
>> -      SmallVector<Value*, 8> NewIndices;
>> -      if (FindElementAtOffset(OrigBase->getType(), Offset.getSExtValue(),
>> -                              NewIndices)) {
>> -        // FIXME: This codepath is completely untested - could be
>> unreachable
>> -        // for all I know.
>> -        // If we were able to index down into an element, create the GEP
>> -        // and bitcast the result.  This eliminates one bitcast,
>> potentially
>> -        // two.
>> -        Value *NGEP = cast<GEPOperator>(GEP)->isInBounds() ?
>> -          Builder->CreateInBoundsGEP(OrigBase, NewIndices) :
>> -          Builder->CreateGEP(OrigBase, NewIndices);
>> -        NGEP->takeName(GEP);
>> -
>> -        if (isa<BitCastInst>(CI))
>> -          return new BitCastInst(NGEP, CI.getType());
>> -        assert(isa<PtrToIntInst>(CI));
>> -        return new PtrToIntInst(NGEP, CI.getType());
>> -      }
>> -    }
>> +    SmallVector<Value *, 8> NewIndices;
>> +    assert(!BCI || !GEP->hasOneUse() ||
>> +           !GEP->accumulateConstantOffset(DL, Offset) ||
>> +           !FindElementAtOffset(BCI->getOperand(0)->getType(),
>> +                                Offset.getSExtValue(), NewIndices));
>> +#endif
>>    }
>>
>>    return commonCastTransforms(CI);
>>
>> Modified: llvm/trunk/test/Transforms/InstCombine/2007-05-14-Crash.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/2007-05-14-Crash.ll?rev=233116&r1=233115&r2=233116&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/InstCombine/2007-05-14-Crash.ll (original)
>> +++ llvm/trunk/test/Transforms/InstCombine/2007-05-14-Crash.ll Tue Mar 24
>> 16:31:31 2015
>> @@ -15,4 +15,10 @@ entry:
>>          ret i8* %tmp35
>>  }
>>
>> -
>> +define i32* @bar(%struct.abc* %abc) {
>> +entry:
>> +        %tmp1 = bitcast %struct.abc* %abc to %struct.def*
>> +        %tmp3 = getelementptr %struct.def, %struct.def* %tmp1, i32 0, i32
>> 1
>> +        %tmp35 = bitcast %struct.abc* %tmp3 to i32*
>> +        ret i32* %tmp35
>> +}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>