[llvm-commits] [llvm] r168361 - in /llvm/trunk: lib/Transforms/Scalar/SROA.cpp test/Transforms/SROA/phi-and-select.ll

[32bitmicro]

> Pawel, another simple bug fix for 3.2 I think. I'm the code owner for SROA.


r168361, r168346 and r168227 merged into 3.2 branch release as
single SROA changset in 168443.

Committed revision 168443.


> Re: [llvm-commits] [llvm] r168346 ...
>
> Pawel, please pull this patch (and if necessary Evan's prior patch,
> r168227) into the 3.2 release branch. They fix quite common crashes.

Indeed the r168227 was necessary for r168346.


> 
> On Tue, Nov 20, 2012 at 2:02 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
>> Author: chandlerc
>> Date: Tue Nov 20 04:02:19 2012
>> New Revision: 168361
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=168361&view=rev
>> Log:
>> Fix PR14132 and handle OOB loads speculated throuh PHI nodes.
>>
>> The issue is that we may end up with newly OOB loads when speculating
>> a load into the predecessors of a PHI node, and this confuses the new
>> integer splitting logic in some cases, triggering an assertion failure.
>> In fact, the branch in question must be dead code as it loads from
>> a too-narrow alloca. Add code to handle this gracefully and leave the
>> requisite FIXMEs for both optimizing more aggressively and doing more to
>> aid sanitizing invalid code which triggers these patterns.
>>
>> Modified:
>>     llvm/trunk/lib/Transforms/Scalar/SROA.cpp
>>     llvm/trunk/test/Transforms/SROA/phi-and-select.ll
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/SROA.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SROA.cpp?rev=168361&r1=168360&r2=168361&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/SROA.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/SROA.cpp Tue Nov 20 04:02:19 2012
>> @@ -568,6 +568,10 @@
>>
>>      // Clamp the end offset to the end of the allocation. Note that this is
>>      // formulated to handle even the case where "BeginOffset + Size" overflows.
>> +    // NOTE! This may appear superficially to be something we could ignore
>> +    // entirely, but that is not so! There may be PHI-node uses where some
>> +    // instructions are dead but not others. We can't completely ignore the
>> +    // PHI node, and so have to record at least the information here.
>>      assert(AllocSize >= BeginOffset); // Established above.
>>      if (Size > AllocSize - BeginOffset) {
>>        DEBUG(dbgs() << "WARNING: Clamping a " << Size << " byte use @" << Offset
>> @@ -2492,6 +2496,23 @@
>>
>>      uint64_t Size = EndOffset - BeginOffset;
>>      bool IsSplitIntLoad = Size < TD.getTypeStoreSize(LI.getType());
>> +
>> +    // If this memory access can be shown to *statically* extend outside the
>> +    // bounds of the original allocation it's behavior is undefined. Rather
>> +    // than trying to transform it, just replace it with undef.
>> +    // FIXME: We should do something more clever for functions being
>> +    // instrumented by asan.
>> +    // FIXME: Eventually, once ASan and friends can flush out bugs here, this
>> +    // should be transformed to a load of null making it unreachable.
>> +    uint64_t OldAllocSize = TD.getTypeAllocSize(OldAI.getAllocatedType());
>> +    if (TD.getTypeStoreSize(LI.getType()) > OldAllocSize) {
>> +      LI.replaceAllUsesWith(UndefValue::get(LI.getType()));
>> +      Pass.DeadInsts.insert(&LI);
>> +      deleteIfTriviallyDead(OldOp);
>> +      DEBUG(dbgs() << "          to: undef!!\n");
>> +      return true;
>> +    }
>> +
>>      Type *TargetTy = IsSplitIntLoad ? Type::getIntNTy(LI.getContext(), Size * 8)
>>                                      : LI.getType();
>>      bool IsPtrAdjusted = false;
>>
>> Modified: llvm/trunk/test/Transforms/SROA/phi-and-select.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SROA/phi-and-select.ll?rev=168361&r1=168360&r2=168361&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/SROA/phi-and-select.ll (original)
>> +++ llvm/trunk/test/Transforms/SROA/phi-and-select.ll Tue Nov 20 04:02:19 2012
>> @@ -390,3 +390,38 @@
>>    %tmpcast.d.0 = select i1 undef, i32* %c, i32* %d.0
>>    br label %for.cond
>>  }
>> +
>> +define i64 @PR14132(i1 %flag) {
>> +; CHECK: @PR14132
>> +; Here we form a PHI-node by promoting the pointer alloca first, and then in
>> +; order to promote the other two allocas, we speculate the load of the
>> +; now-phi-node-pointer. In doing so we end up loading a 64-bit value from an i8
>> +; alloca, which is completely bogus. However, we were asserting on trying to
>> +; rewrite it. Now it is replaced with undef. Eventually we may replace it with
>> +; unrechable and even the CFG will go away here.
>> +entry:
>> +  %a = alloca i64
>> +  %b = alloca i8
>> +  %ptr = alloca i64*
>> +; CHECK-NOT: alloca
>> +
>> +  %ptr.cast = bitcast i64** %ptr to i8**
>> +  store i64 0, i64* %a
>> +  store i8 1, i8* %b
>> +  store i64* %a, i64** %ptr
>> +  br i1 %flag, label %if.then, label %if.end
>> +
>> +if.then:
>> +  store i8* %b, i8** %ptr.cast
>> +  br label %if.end
>> +
>> +if.end:
>> +  %tmp = load i64** %ptr
>> +  %result = load i64* %tmp
>> +; CHECK-NOT: store
>> +; CHECK-NOT: load
>> +; CHECK: %[[result:.*]] = phi i64 [ undef, %if.then ], [ 0, %entry ]
>> +
>> +  ret i64 %result
>> +; CHECK-NEXT: ret i64 %[[result]]
>> +}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
>