[X86] Teach how to custom lower double-to-half conversions under fast-math.

[andreadb]

Summary

This patch teaches the backend how to custom lower a 'fp_to_fp16' node that performs a double-to-half conversion.

Under fast-math, if the target has F16C, the backend can expand a double-to-half conversion into a double-to-float conversion immediately followed by a float-to-half conversion. Before this patch, a double-to-half conversion was always expanded into a library call even under fast-math.

Example:
\code
define zeroext i16 @func(double %d) #0 {
entry:

%0 = tail call i16 @llvm.convert.to.fp16.f64(double %d)
ret i16 %0

}

attributes #0 = { "unsafe-fp-math=true" "use-soft-float"="false" }
\code end

Before this patch (with -mattr=+f16c), the conversion from double to fp16 was expanded into a library call to function '__truncdfhf2'.

With this patch, the double-to-half conversion is now expanded into the sequence:

vcvtsd2ss %xmm0, %xmm0, %xmm0
vcvtps2ph $0, %xmm0, %xmm0

Note that this patch also handles 'long double'-to-half conversions.

This patch doesn't add custom lowering rules for 'fp16_to_fp' dag nodes. The reason why we don't need those rules is because LegalizeDAG (see around lines 3532:3546) already knows how to expand a half-to-double conversion into a 'FP16_TO_FP' plus 'FP_EXTEND'.

Please let me know if ok to submit.

Thanks!
Andrea

[ab]

Should this be in the target-independent legalizer instead, like FP16_TO_FP? Only with UnsafeFPMath, of course (I'm a bit uncomfortable with that, but I don't find it shocking, and I see there's precedent.)

Otherwise, the change seems reasonable, thanks!

-Ahmed

test/CodeGen/X86/fastmath-float-half-conversion.ll

17

Not very important, but add a check for fp80->fp32, perhaps?

[andreadb]

Hi Ahmed,

Thanks for the review!
Here is a updated patch. As you suggested, I moved the expansion of FP_TO_FP16 into the target independed dag legalizer.
I also updated the test adding extra CHECK lines for the 'long double-to-float' conversion.

The 'long double-to-float conversion' is currently implemented by the sequence: fldt+fstps+movss.
Basically, the long double in input to the function is firstly pushed onto the top of the x87 FPU register stack, and then 'popped' from the FPU stack onto the stack again. Finally, it is loaded as a float using a zero extending movss. The code sequence looks a bit redundant but it works :-)

Please let me know what you think.

Thanks!
-Andrea

[ab]

In D7832#128399, @andreadb wrote:

Hi Ahmed,

Thanks for the review!
Here is a updated patch. As you suggested, I moved the expansion of FP_TO_FP16 into the target independed dag legalizer.

If f16->f32 isn't legal as well, we just turned one libcall into two, no?

I also updated the test adding extra CHECK lines for the 'long double-to-float' conversion.

The 'long double-to-float conversion' is currently implemented by the sequence: fldt+fstps+movss.
Basically, the long double in input to the function is firstly pushed onto the top of the x87 FPU register stack, and then 'popped' from the FPU stack onto the stack again. Finally, it is loaded as a float using a zero extending movss. The code sequence looks a bit redundant but it works :-)

I expected something like that, thanks for adding it!

-Ahmed

Please let me know what you think.

Thanks!
-Andrea

[andreadb]

In D7832#128423, @ab wrote:

In D7832#128399, @andreadb wrote:

Hi Ahmed,

Thanks for the review!
Here is a updated patch. As you suggested, I moved the expansion of FP_TO_FP16 into the target independed dag legalizer.

If f16->f32 isn't legal as well, we just turned one libcall into two, no?

Right, it doesn't make sense to expand that node if f16->f32 is not legal.
We would still generate a single libcall (for the f16->f32 conversion). However, we would get a extra (v)cvtsd2ss for the double-half conversion.
I will add a check to make sure that the f16->f32 is 'legalOrCustom' before attempting to expand the double-half conversion.

I will upload a new version patch.

I also updated the test adding extra CHECK lines for the 'long double-to-float' conversion.

The 'long double-to-float conversion' is currently implemented by the sequence: fldt+fstps+movss.
Basically, the long double in input to the function is firstly pushed onto the top of the x87 FPU register stack, and then 'popped' from the FPU stack onto the stack again. Finally, it is loaded as a float using a zero extending movss. The code sequence looks a bit redundant but it works :-)

I expected something like that, thanks for adding it!

-Ahmed

Please let me know what you think.

Thanks!
-Andrea

[andreadb]

Uploaded a new version of the patch.
This time, we avoid expanding a 'fp_to_fp16' if the float-half is not legal (or custom) for the target.
I also added a extra RUN line in my new test to verify the codegen if the target doesn't have F16C.

Please let me know what you think.

Thanks again for your time.
Andrea

[ab]

LGTM now, thanks!

-Ahmed

[andreadb]

Thanks Ahmed.
Committed revision 230276.

lib/Target/X86/X86ISelLowering.cpp

X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
   }
 
   // Special handling for half-precision floating point conversions.
   // If we don't have F16C support, then lower half float conversions
   // into library calls.
   if (TM.Options.UseSoftFloat || !Subtarget->hasF16C()) {
     setOperationAction(ISD::FP16_TO_FP, MVT::f32, Expand);
     setOperationAction(ISD::FP_TO_FP16, MVT::f32, Expand);
+    setOperationAction(ISD::FP_TO_FP16, MVT::f64, Expand);
+    setOperationAction(ISD::FP_TO_FP16, MVT::f80, Expand);
+  } else {
+    setOperationAction(ISD::FP_TO_FP16, MVT::f64, Custom);
+    setOperationAction(ISD::FP_TO_FP16, MVT::f80, Custom);
   }
 
-  // There's never any support for operations beyond MVT::f32.
   setOperationAction(ISD::FP16_TO_FP, MVT::f64, Expand);
   setOperationAction(ISD::FP16_TO_FP, MVT::f80, Expand);
-  setOperationAction(ISD::FP_TO_FP16, MVT::f64, Expand);
-  setOperationAction(ISD::FP_TO_FP16, MVT::f80, Expand);
-
   setLoadExtAction(ISD::EXTLOAD, MVT::f32, MVT::f16, Expand);
   setLoadExtAction(ISD::EXTLOAD, MVT::f64, MVT::f16, Expand);
   setLoadExtAction(ISD::EXTLOAD, MVT::f80, MVT::f16, Expand);
   setTruncStoreAction(MVT::f32, MVT::f16, Expand);
   setTruncStoreAction(MVT::f64, MVT::f16, Expand);
   setTruncStoreAction(MVT::f80, MVT::f16, Expand);
 
   if (Subtarget->hasPOPCNT()) {
static SDValue LowerADDC_ADDE_SUBC_SUBE(SDValue Op, SelectionDAG &DAG) {
 
   if (!ExtraOp)
     return DAG.getNode(Opc, SDLoc(Op), VTs, Op.getOperand(0),
                        Op.getOperand(1));
   return DAG.getNode(Opc, SDLoc(Op), VTs, Op.getOperand(0),
                      Op.getOperand(1), Op.getOperand(2));
 }
 
+static SDValue LowerFP_TO_FP16(SDValue Op, const X86Subtarget *Subtarget,
+                            SelectionDAG &DAG) {
+  assert(Subtarget->hasF16C() && "This expansion requires feature F16C!");
+
+  if (!DAG.getTarget().Options.UnsafeFPMath)
+    // Let the backend expand this node into a library call.
+    return SDValue();
+
+  // Under fastmath, we can expand this node into a fround (from f64/f80 to f32)
+  // followed by a float-half conversion.
+  SDValue Op0 = Op.getOperand(0);
+  assert((Op0.getSimpleValueType() == MVT::f64 ||
+          Op0.getSimpleValueType() == MVT::f80) && "Unexpected value type!");
+  SDValue FloatVal = DAG.getNode(ISD::FP_ROUND, SDLoc(Op), MVT::f32, Op0,
+                                 DAG.getIntPtrConstant(0));
+  return DAG.getNode(ISD::FP_TO_FP16, SDLoc(Op), MVT::i16, FloatVal);
+}
+
 static SDValue LowerFSINCOS(SDValue Op, const X86Subtarget *Subtarget,
                             SelectionDAG &DAG) {
   assert(Subtarget->isTargetDarwin() && Subtarget->is64Bit());
 
   // For MacOSX, we want to call an alternative entry point: __sincos_stret,
   // which returns the values as { float, float } (in XMM0) or
   // { double, double } (which is returned in XMM0, XMM1).
   SDLoc dl(Op);
SDValue X86TargetLowering::LowerOperation(SDValue Op, SelectionDAG &DAG) const {
   case ISD::SINT_TO_FP:         return LowerSINT_TO_FP(Op, DAG);
   case ISD::UINT_TO_FP:         return LowerUINT_TO_FP(Op, DAG);
   case ISD::TRUNCATE:           return LowerTRUNCATE(Op, DAG);
   case ISD::ZERO_EXTEND:        return LowerZERO_EXTEND(Op, Subtarget, DAG);
   case ISD::SIGN_EXTEND:        return LowerSIGN_EXTEND(Op, Subtarget, DAG);
   case ISD::ANY_EXTEND:         return LowerANY_EXTEND(Op, Subtarget, DAG);
   case ISD::FP_TO_SINT:         return LowerFP_TO_SINT(Op, DAG);
   case ISD::FP_TO_UINT:         return LowerFP_TO_UINT(Op, DAG);
+  case ISD::FP_TO_FP16:         return LowerFP_TO_FP16(Op, Subtarget, DAG);
   case ISD::FP_EXTEND:          return LowerFP_EXTEND(Op, DAG);
   case ISD::LOAD:               return LowerExtendedLoad(Op, Subtarget, DAG);
   case ISD::FABS:
   case ISD::FNEG:               return LowerFABSorFNEG(Op, DAG);
   case ISD::FCOPYSIGN:          return LowerFCOPYSIGN(Op, DAG);
   case ISD::FGETSIGN:           return LowerFGETSIGN(Op, DAG);
   case ISD::SETCC:              return LowerSETCC(Op, DAG);
   case ISD::SELECT:             return LowerSELECT(Op, DAG);

test/CodeGen/X86/fastmath-float-half-conversion.ll

+; RUN: llc -mtriple=x86_64-unknown-unknown -mattr=+f16c < %s | FileCheck %s
+
+define zeroext i16 @test1_fast(double %d) #0 {
+; CHECK-LABEL: test1_fast:
+; CHECK-NOT: callq  {{_+}}truncdfhf2
+; CHECK: vcvtsd2ss %xmm0, %xmm0, %xmm0
+; CHECK-NEXT: vcvtps2ph $0, %xmm0, %xmm0
+; CHECK: ret
+entry:
+  %0 = tail call i16 @llvm.convert.to.fp16.f64(double %d)
+  ret i16 %0
+}
+
+define zeroext i16 @test2_fast(x86_fp80 %d) #0 {
+; CHECK-LABEL: test2_fast:
+; CHECK-NOT: callq  {{_+}}truncxfhf2
+; CHECK: vcvtps2ph $0, %xmm0, %xmm0
+; CHECK: ret
+entry:
+  %0 = tail call i16 @llvm.convert.to.fp16.f80(x86_fp80 %d)
+  ret i16 %0
+}
+
+define zeroext i16 @test1(double %d) #1 {
+; CHECK-LABEL: test1:
+; CHECK: callq  {{_+}}truncdfhf2
+; CHECK: ret
+entry:
+  %0 = tail call i16 @llvm.convert.to.fp16.f64(double %d)
+  ret i16 %0
+}
+
+define zeroext i16 @test2(x86_fp80 %d) #1 {
+; CHECK-LABEL: test2:
+; CHECK: callq  {{_+}}truncxfhf2
+; CHECK: ret
+entry:
+  %0 = tail call i16 @llvm.convert.to.fp16.f80(x86_fp80 %d)
+  ret i16 %0
+}
+
+declare i16 @llvm.convert.to.fp16.f64(double)
+declare i16 @llvm.convert.to.fp16.f80(x86_fp80)
+
+attributes #0 = { nounwind readnone uwtable "unsafe-fp-math"="true" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone uwtable "unsafe-fp-math"="false" "use-soft-float"="false" }