[llvm-commits] [llvm] r169719 - in /llvm/trunk: lib/Transforms/Scalar/SROA.cpp test/Transforms/SROA/basictest.ll test/Transforms/SROA/big-endian.ll

[Pawel Wodnicki]

Chandler,
> Hey Pawel, please pull this into the release branch. It fixes a release
> blocker PR filed against one of the RCs.

Good one, thank you!
Committed revision 169735.


> 
> (I'm the code owner for SROA fwiw...)
> 
> 
> On Sun, Dec 9, 2012 at 4:54 PM, Chandler Carruth <chandlerc at gmail.com>wrote:
> 
>> Author: chandlerc
>> Date: Sun Dec  9 18:54:45 2012
>> New Revision: 169719
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=169719&view=rev
>> Log:
>> Fix PR14548: SROA was crashing on a mixture of i1 and i8 loads and stores.
>>
>> When SROA was evaluating a mixture of i1 and i8 loads and stores, in
>> just a particular case, it would tickle a latent bug where we compared
>> bits to bytes rather than bits to bits. As a consequence of the latent
>> bug, we would allow integers through which were not byte-size multiples,
>> a situation the later rewriting code was never intended to handle.
>>
>> In release builds this could trigger all manner of oddities, but the
>> reported issue in PR14548 was forming invalid bitcast instructions.
>>
>> The only downside of this fix is that it makes it more clear that SROA
>> in its current form is not capable of handling mixed i1 and i8 loads and
>> stores. Sometimes with the previous code this would work by luck, but
>> usually it would crash, so I'm not terribly worried. I'll watch the LNT
>> numbers just to be sure.
>>
>> Modified:
>>     llvm/trunk/lib/Transforms/Scalar/SROA.cpp
>>     llvm/trunk/test/Transforms/SROA/basictest.ll
>>     llvm/trunk/test/Transforms/SROA/big-endian.ll
>>
>> Modified: llvm/trunk/lib/Transforms/Scalar/SROA.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/SROA.cpp?rev=169719&r1=169718&r2=169719&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Scalar/SROA.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Scalar/SROA.cpp Sun Dec  9 18:54:45 2012
>> @@ -2184,7 +2184,7 @@
>>        if (RelBegin == 0 && RelEnd == Size)
>>          WholeAllocaOp = true;
>>        if (IntegerType *ITy = dyn_cast<IntegerType>(LI->getType())) {
>> -        if (ITy->getBitWidth() < TD.getTypeStoreSize(ITy))
>> +        if (ITy->getBitWidth() < TD.getTypeStoreSizeInBits(ITy))
>>            return false;
>>          continue;
>>        }
>> @@ -2200,7 +2200,7 @@
>>        if (RelBegin == 0 && RelEnd == Size)
>>          WholeAllocaOp = true;
>>        if (IntegerType *ITy = dyn_cast<IntegerType>(ValueTy)) {
>> -        if (ITy->getBitWidth() < TD.getTypeStoreSize(ITy))
>> +        if (ITy->getBitWidth() < TD.getTypeStoreSizeInBits(ITy))
>>            return false;
>>          continue;
>>        }
>>
>> Modified: llvm/trunk/test/Transforms/SROA/basictest.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SROA/basictest.ll?rev=169719&r1=169718&r2=169719&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/SROA/basictest.ll (original)
>> +++ llvm/trunk/test/Transforms/SROA/basictest.ll Sun Dec  9 18:54:45 2012
>> @@ -1147,3 +1147,32 @@
>>    ret void
>>  ; CHECK: ret
>>  }
>> +
>> +define void @PR14548(i1 %x) {
>> +; Handle a mixture of i1 and i8 loads and stores to allocas. This
>> particular
>> +; pattern caused crashes and invalid output in the PR, and its nature will
>> +; trigger a mixture in several permutations as we resolve each alloca
>> +; iteratively.
>> +; Note that we don't do a particularly good *job* of handling these
>> mixtures,
>> +; but the hope is that this is very rare.
>> +; CHECK: @PR14548
>> +
>> +entry:
>> +  %a = alloca <{ i1 }>, align 8
>> +  %b = alloca <{ i1 }>, align 8
>> +; Nothing of interest is simplified here.
>> +; CHECK: alloca
>> +; CHECK: alloca
>> +
>> +  %b.i1 = bitcast <{ i1 }>* %b to i1*
>> +  store i1 %x, i1* %b.i1, align 8
>> +  %b.i8 = bitcast <{ i1 }>* %b to i8*
>> +  %foo = load i8* %b.i8, align 1
>> +
>> +  %a.i8 = bitcast <{ i1 }>* %a to i8*
>> +  call void @llvm.memcpy.p0i8.p0i8.i32(i8* %a.i8, i8* %b.i8, i32 1, i32
>> 1, i1 false) nounwind
>> +  %bar = load i8* %a.i8, align 1
>> +  %a.i1 = getelementptr inbounds <{ i1 }>* %a, i32 0, i32 0
>> +  %baz = load i1* %a.i1, align 1
>> +  ret void
>> +}
>>
>> Modified: llvm/trunk/test/Transforms/SROA/big-endian.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SROA/big-endian.ll?rev=169719&r1=169718&r2=169719&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/SROA/big-endian.ll (original)
>> +++ llvm/trunk/test/Transforms/SROA/big-endian.ll Sun Dec  9 18:54:45 2012
>> @@ -82,14 +82,9 @@
>>
>>    %a0i16ptr = bitcast i8* %a0ptr to i16*
>>    store i16 1, i16* %a0i16ptr
>> -; CHECK:      %[[mask0:.*]] = and i16 1, -16
>> -
>> -  %a1i4ptr = bitcast i8* %a1ptr to i4*
>> -  store i4 1, i4* %a1i4ptr
>> -; CHECK-NEXT: %[[insert0:.*]] = or i16 %[[mask0]], 1
>>
>>    store i8 1, i8* %a2ptr
>> -; CHECK-NEXT: %[[mask1:.*]] = and i40 undef, 4294967295
>> +; CHECK:      %[[mask1:.*]] = and i40 undef, 4294967295
>>  ; CHECK-NEXT: %[[insert1:.*]] = or i40 %[[mask1]], 4294967296
>>
>>    %a3i24ptr = bitcast i8* %a3ptr to i24*
>> @@ -110,7 +105,7 @@
>>    %ai = load i56* %aiptr
>>    %ret = zext i56 %ai to i64
>>    ret i64 %ret
>> -; CHECK-NEXT: %[[ext4:.*]] = zext i16 %[[insert0]] to i56
>> +; CHECK-NEXT: %[[ext4:.*]] = zext i16 1 to i56
>>  ; CHECK-NEXT: %[[shift4:.*]] = shl i56 %[[ext4]], 40
>>  ; CHECK-NEXT: %[[mask4:.*]] = and i56 %[[insert3]], 1099511627775
>>  ; CHECK-NEXT: %[[insert4:.*]] = or i56 %[[mask4]], %[[shift4]]
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>