[llvm] r221820 - [CodeGenPrepare][AArch64] Fix a TLI legality check on iPTR to use a lowered instead.

[Ahmed Bougacha]

On Wed, Nov 12, 2014 at 2:35 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> Hi Ahmed,
>
> We have other uses of this construction in CodeGenPrepare, shouldn’t we fix those too?

Yes, I was actually looking at them to try and come up with testcases.

There is one result of EVT::getEVT() that is passed to
allowsMisalignedMemoryAccesses, and that one is the target's
responsibility, and wouldn't hit the assert anyway.

There are two that involve TLI legality checks. For both I don't think
it's possible for them to fail as is:
- one only looks at binary operators, and we don't have any of those
on pointers IIRC.
- the other looks at extensions only

I'm changing them just in case (for the first, there's actually a
comment saying that we should also look at cast operators, and in that
case we will hit the same problem), but they won't have testcases.

Anyway, thanks for having a look, I'm committing the other changes as we speak.

-Ahmed

> Thanks,
> Quentin
>
> On Nov 12, 2014, at 2:16 PM, Ahmed Bougacha <ahmed.bougacha at gmail.com> wrote:
>
>> Author: ab
>> Date: Wed Nov 12 16:16:55 2014
>> New Revision: 221820
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=221820&view=rev
>> Log:
>> [CodeGenPrepare][AArch64] Fix a TLI legality check on iPTR to use a lowered instead.
>>
>> Fixes PR21548.  Related to PR20474.
>>
>>
>> Modified:
>>    llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
>>    llvm/trunk/test/Transforms/CodeGenPrepare/AArch64/trunc-weird-user.ll
>>
>> Modified: llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp?rev=221820&r1=221819&r2=221820&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp Wed Nov 12 16:16:55 2014
>> @@ -682,8 +682,8 @@ SinkShiftAndTruncate(BinaryOperator *Shi
>>     // FIXME: always querying the result type is just an
>>     // approximation; some nodes' legality is determined by the
>>     // operand or other means. There's no good way to find out though.
>> -    if (TLI.isOperationLegalOrCustom(ISDOpcode,
>> -                                     EVT::getEVT(TruncUser->getType(), true)))
>> +    if (TLI.isOperationLegalOrCustom(
>> +            ISDOpcode, TLI.getValueType(TruncUser->getType(), true)))
>>       continue;
>>
>>     // Don't bother for PHI nodes.
>>
>> Modified: llvm/trunk/test/Transforms/CodeGenPrepare/AArch64/trunc-weird-user.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeGenPrepare/AArch64/trunc-weird-user.ll?rev=221820&r1=221819&r2=221820&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/CodeGenPrepare/AArch64/trunc-weird-user.ll (original)
>> +++ llvm/trunk/test/Transforms/CodeGenPrepare/AArch64/trunc-weird-user.ll Wed Nov 12 16:16:55 2014
>> @@ -15,3 +15,22 @@ define %foo @test_merge(i32 %in) {
>>   %struct = insertvalue %foo undef, i8 %byte, 0
>>   ret %"foo" %struct
>> }
>> +
>> +define i64* @test_merge_PR21548(i32 %a, i64* %p1, i64* %p2, i64* %p3) {
>> +; CHECK-LABEL: @test_merge_PR21548
>> +  %as = lshr i32 %a, 3
>> +  %Tr = trunc i32 %as to i1
>> +  br i1 %Tr, label %BB2, label %BB3
>> +
>> +BB2:
>> +  ; Similarly to above:
>> +  ; CodeGenPrepare was requesting the EVT for i8* to determine
>> +  ; whether the select user of the trunc was legal. This asserted.
>> +
>> +; CHECK: select i1 {{%.*}}, i64* %p1, i64* %p2
>> +  %p = select i1 %Tr, i64* %p1, i64* %p2
>> +  ret i64* %p
>> +
>> +BB3:
>> +  ret i64* %p3
>> +}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>