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

[David Blaikie]

On Tue, Mar 24, 2015 at 4:16 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> Looks like this was
>
> https://llvm.org/bugs/show_bug.cgi?id=23010


As in this commit caused that failure as well as the sanitizer one? Hmm.
Thanks for the pointer - I'll have to have a look at that...


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