[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]

Test case added in r282473

~Craig

On Mon, Sep 26, 2016 at 7:02 PM, Craig Topper <craig.topper at gmail.com>
wrote:

> 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/825dbffb/attachment.html>