From ef9962d9bbbf4218933bea09e217b36822e029d1 Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Tue, 24 Mar 2015 21:50:35 +0000 Subject: [PATCH] Revert "Remove an InstCombine that seems to have become redundant." Assertion fires in compiler-rt. Guess it does fire.. This reverts commit r233116. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@233121 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../InstCombine/InstCombineCasts.cpp | 41 +++++++++++++------ .../InstCombine/2007-05-14-Crash.ll | 8 +--- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/lib/Transforms/InstCombine/InstCombineCasts.cpp b/lib/Transforms/InstCombine/InstCombineCasts.cpp index 94883cab12a..0625d8deca0 100644 --- a/lib/Transforms/InstCombine/InstCombineCasts.cpp +++ b/lib/Transforms/InstCombine/InstCombineCasts.cpp @@ -1455,20 +1455,35 @@ 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. - // 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); + unsigned AS = GEP->getPointerAddressSpace(); + unsigned OffsetBits = DL.getPointerSizeInBits(AS); + APInt Offset(OffsetBits, 0); BitCastInst *BCI = dyn_cast(GEP->getOperand(0)); - SmallVector NewIndices; - assert(!BCI || !GEP->hasOneUse() || - !GEP->accumulateConstantOffset(DL, Offset) || - !FindElementAtOffset(BCI->getOperand(0)->getType(), - Offset.getSExtValue(), NewIndices)); -#endif + 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()); + } + } } return commonCastTransforms(CI); diff --git a/test/Transforms/InstCombine/2007-05-14-Crash.ll b/test/Transforms/InstCombine/2007-05-14-Crash.ll index 056ff2cf826..a3d21659143 100644 --- a/test/Transforms/InstCombine/2007-05-14-Crash.ll +++ b/test/Transforms/InstCombine/2007-05-14-Crash.ll @@ -15,10 +15,4 @@ 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 -} + -- 2.34.1