[llvm] r282341 - [X86] Teach combineShuffle to avoid creating floating point operations with integer types and integer operations with floating point types. Seems isOperationLegal lies for mismatched types and operations.

[Craig Topper via llvm-commits]

I didn't have a small test case. The bug report had pretty large IR. I'll
see if I can reduce it.

I'm not very familiar with this code. Adding Andrea who originally wrote
the code and Simon who has been doing a lot shuffle combining change
recently.

~Craig

On Mon, Sep 26, 2016 at 12:22 PM, Friedman, Eli <efriedma at codeaurora.org>
wrote:

> On 9/24/2016 2:42 PM, Craig Topper via llvm-commits wrote:
>
>> Author: ctopper
>> Date: Sat Sep 24 16:42:49 2016
>> New Revision: 282341
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=282341&view=rev
>> Log:
>> [X86] Teach combineShuffle to avoid creating floating point operations
>> with integer types and integer operations with floating point types. Seems
>> isOperationLegal lies for mismatched types and operations.
>>
>> Fixes PR30511.
>>
>> Modified:
>>      llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>>
>
> Missing testcase.
>
>> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X8
>> 6/X86ISelLowering.cpp?rev=282341&r1=282340&r2=282341&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Sat Sep 24 16:42:49
>> 2016
>> @@ -26424,13 +26424,18 @@ static SDValue combineShuffle(SDNode *N,
>>         bool CanFold = false;
>>         switch (Opcode) {
>>         default : break;
>> -      case ISD::ADD :
>> -      case ISD::FADD :
>> -      case ISD::SUB :
>> -      case ISD::FSUB :
>> -      case ISD::MUL :
>> -      case ISD::FMUL :
>> -        CanFold = true;
>> +      case ISD::ADD:
>> +      case ISD::SUB:
>> +      case ISD::MUL:
>> +        // isOperationLegal lies for integer ops on floating point types.
>> +        CanFold = VT.isInteger();
>> +        break;
>> +      case ISD::FADD:
>> +      case ISD::FSUB:
>> +      case ISD::FMUL:
>> +        // isOperationLegal lies for floating point ops on integer types.
>> +        CanFold = VT.isFloatingPoint();
>> +        break;
>>
>
> Why are we checking for FADD here in the first place?  This transformation
> isn't legal for floating-point numbers.
>
> Also, it's legal to transform ADD even if VT is floating-point; we just
> need to come up with an appropriate integer type to use to perform the
> operation, then insert an extra bitcast.
>
> -Eli
>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160926/b8b9f547/attachment.html>