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

[Friedman, Eli via llvm-commits]

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/X86/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