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