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

David Blaikie dblaikie at gmail.com
Tue Mar 24 14:53:37 PDT 2015


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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150324/e0d9c247/attachment.html>


More information about the llvm-commits mailing list