From 80da8623a44ed1c74a03ced93276321cdc8e52b8 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Tue, 24 Mar 2015 21:31:31 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@233116 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../InstCombine/InstCombineCasts.cpp | 41 ++++++------------- .../InstCombine/2007-05-14-Crash.ll | 8 +++- 2 files changed, 20 insertions(+), 29 deletions(-) diff --git a/lib/Transforms/InstCombine/InstCombineCasts.cpp b/lib/Transforms/InstCombine/InstCombineCasts.cpp index 0625d8deca0..94883cab12a 100644 --- a/lib/Transforms/InstCombine/InstCombineCasts.cpp +++ b/lib/Transforms/InstCombine/InstCombineCasts.cpp @@ -1455,35 +1455,20 @@ Instruction *InstCombiner::commonPointerCastTransforms(CastInst &CI) { // 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(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 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(GEP)->isInBounds() ? - Builder->CreateInBoundsGEP(OrigBase, NewIndices) : - Builder->CreateGEP(OrigBase, NewIndices); - NGEP->takeName(GEP); - - if (isa(CI)) - return new BitCastInst(NGEP, CI.getType()); - assert(isa(CI)); - return new PtrToIntInst(NGEP, CI.getType()); - } - } + SmallVector NewIndices; + assert(!BCI || !GEP->hasOneUse() || + !GEP->accumulateConstantOffset(DL, Offset) || + !FindElementAtOffset(BCI->getOperand(0)->getType(), + Offset.getSExtValue(), NewIndices)); +#endif } return commonCastTransforms(CI); diff --git a/test/Transforms/InstCombine/2007-05-14-Crash.ll b/test/Transforms/InstCombine/2007-05-14-Crash.ll index a3d21659143..056ff2cf826 100644 --- a/test/Transforms/InstCombine/2007-05-14-Crash.ll +++ b/test/Transforms/InstCombine/2007-05-14-Crash.ll @@ -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 +}