[DAGCombine] Suppress some foldings of rounding to fp16

[wristow]

Summary

Folding of 'x86_fp80 -> float -> half' into a direct rounding of
'x86_fp80 -> half' is legal when "unsafe-fp-math" is true, but it has
been suppressed for efficiency reasons because it generally results in
a run-time call rather than native instructions. The same applies for
vector versions, but those foldings have not been suppressed. The same
efficiency aspect applies to folding of 'double -> float -> half', but
those foldings have not been suppressed (scalar or vector).

This patch suppresses the foldings for the 'double' source (scalar and
vector), and for the vector version of 'x86_fp80' (to match the scalar
approach).

[wristow]

I intend to commit an NFC version of the updated test that shows the current behavior of main, with TODOs in it indicating the desired changes.

[lebedev.ri]

Sounds like the lowering should just be improved to do the opposite of this?

[wristow]

Sounds like the lowering should just be improved to do the opposite of this?

Thanks for the quick response @lebedev.ri.

So (focusing on the 'double' source) by that, do you mean splitting 'double -> half' into 'double -> float -> half'? That isn't technically safe, although it could be done legally in "unsafe-fp-math" mode. Ironically, that's what used to happen, but 02fe96b24018bb8ce65cb264e0621459507cf989 suppressed that feature. We could modify the change of that commit to no longer suppress it in "unsafe-fp-math" mode.

FTR, that commit 02fe96b24018bb8ce65cb264e0621459507cf989 caused us trouble, in that we encountered user-code that essentially did:

void test(double d, __fp16 *pFp16) {
  *pFp16 = __fp16(float(d));
}

and that previously worked fine even though dagcombine folded this into a single rounding, __fp16(d), because the lowering un-did that folding. With that commit, this folding results in a call to __truncdfhf2 for our target (and some others). And we don't supply that routine in our run-time lib. Seeing the comment in dagcombine:
"f80 to f16 always generates an expensive (and as yet, unimplemented) libcall to __truncxfhf2"
I was motivated to expand the check here to also handle the folding from double (and to have the code here also handle the vector versions).

Until your suggestion, I hadn't considered modifying the change of 02fe96b24018bb8ce65cb264e0621459507cf989 to allow it to do the transform when "unsafe-fp-math" is on. Thinking about it now, that should also generate better code (when "unsafe-fp-math" is on) for the case where the user has explicitly written a conversion from double to __fp16. So that seems better to me. I'll do that, unless others come back with an argument to have this done in dagcombine.

[pengfei]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

16274–16275

This equals to VT.getScalarType() == MVT::f16.

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

68

It's still efficient to in the AVX case. Calling to single run-time is better than vcvtsd2ss + __truncsfhf2.

[wristow]

I'll re-work this to handle it in the lowering, as @lebedev.ri suggested. And take the other comments into account then.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

16274–16275

Thanks @pengfei. (I should have seen that.)

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

68

Good point. When I re-work this to handle things in the lowering, I'll take that into account.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

SDValue DAGCombiner::visitFP_ROUND(SDNode *N) {
   if (N0.getOpcode() == ISD::FP_EXTEND && VT == N0.getOperand(0).getValueType())
     return N0.getOperand(0);
 
   // fold (fp_round (fp_round x)) -> (fp_round x)
   if (N0.getOpcode() == ISD::FP_ROUND) {
     const bool NIsTrunc = N->getConstantOperandVal(1) == 1;
     const bool N0IsTrunc = N0.getConstantOperandVal(1) == 1;
 
-    // Skip this folding if it results in an fp_round from f80 to f16.
+    // Skip this folding if it results in an fp_round from f80 or f64 to f16.
     //
-    // f80 to f16 always generates an expensive (and as yet, unimplemented)
-    // libcall to __truncxfhf2 instead of selecting native f16 conversion
-    // instructions from f32 or f64.  Moreover, the first (value-preserving)
-    // fp_round from f80 to either f32 or f64 may become a NOP in platforms like
-    // x86.
-    if (N0.getOperand(0).getValueType() == MVT::f80 && VT == MVT::f16)
+    // f80 (or f64) to f16 always generate an expensive (and as yet,
+    // unimplemented on most targets) libcall to __truncxfhf2 (or to
+    // __truncdfhf2) instead of selecting native f16 conversion instructions
+    // from f32.  Suppress this folding in both the scalar and vector forms.
+    bool TargIsFp16 = VT == MVT::f16 ||
+                      (VT.isVector() && VT.getVectorElementType() == MVT::f16);
+    if (TargIsFp16) {
+      EVT SrcVT = N0.getOperand(0).getValueType();
+      if (SrcVT.isVector())
+        SrcVT = SrcVT.getVectorElementType();
+      if (SrcVT == MVT::f64 || SrcVT == MVT::f80)
         return SDValue();
+    }
 
     // If the first fp_round isn't a value preserving truncation, it might
     // introduce a tie in the second fp_round, that wouldn't occur in the
     // single-step fp_round we want to fold to.
     // In other words, double rounding isn't the same as rounding.
     // Also, this is a value preserving truncation iff both fp_round's are.
     if (DAG.getTarget().Options.UnsafeFPMath || N0IsTrunc) {
       SDLoc DL(N);

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

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=x86_64-unknown-unknown -mattr=+f16c < %s | FileCheck %s --check-prefix=ALL
-; RUN: llc -mtriple=x86_64-unknown-unknown -mattr=+avx < %s | FileCheck %s --check-prefix=ALL
+; RUN: llc -mtriple=x86_64-unknown-unknown -mattr=+f16c < %s | FileCheck %s --check-prefix=ALL --check-prefix=F16C
+; RUN: llc -mtriple=x86_64-unknown-unknown -mattr=+avx < %s | FileCheck %s --check-prefix=ALL --check-prefix=AVX
 
 define zeroext i16 @test1_fast(double %d) #0 {
 ; ALL-LABEL: test1_fast:
 ; ALL:       # %bb.0: # %entry
 ; ALL-NEXT:    pushq %rax
 ; ALL-NEXT:    callq __truncdfhf2@PLT
 ; ALL-NEXT:    vpextrw $0, %xmm0, %eax
 ; ALL-NEXT:    # kill: def $ax killed $ax killed $eax

 ; ALL-NEXT:    # kill: def $ax killed $ax killed $eax
 ; ALL-NEXT:    addq $24, %rsp
 ; ALL-NEXT:    retq
 entry:
   %0 = tail call i16 @llvm.convert.to.fp16.f80(x86_fp80 %d)
   ret i16 %0
 }
 
+; The 'test3*' versions convert:
+;   'double' -> 'float' -> 'half'
+; With 'unsafe-fp-math', it is legal to fold this to converting directly
+; from 'double' -> 'half'.  But that is generally less efficient (done
+; via slow run-time calls) than the two-step unfolded approach.  So
+; verify that we generate the same code (without the folding) for
+; the plain and fast versions.
+define half @test3_fast(double %d) #0 {
+; F16C-LABEL: test3_fast:
+; F16C:       # %bb.0:
+; F16C-NEXT:    vcvtsd2ss %xmm0, %xmm0, %xmm0
+; F16C-NEXT:    vcvtps2ph $4, %xmm0, %xmm0
+; F16C-NEXT:    vmovd %xmm0, %eax
+; F16C-NEXT:    vpinsrw $0, %eax, %xmm0, %xmm0
+; F16C-NEXT:    retq
+;
+; AVX-LABEL: test3_fast:
+; AVX:       # %bb.0:
+; AVX-NEXT:    pushq %rax
+; AVX-NEXT:    vcvtsd2ss %xmm0, %xmm0, %xmm0
+; AVX-NEXT:    callq __truncsfhf2@PLT
+; AVX-NEXT:    popq %rax
+; AVX-NEXT:    retq
+  %1 = fptrunc double %d to float
+  %2 = fptrunc float %1 to half
+  ret half %2
+}
+
+define half @test3(double %d) #1 {
+; F16C-LABEL: test3:
+; F16C:       # %bb.0:
+; F16C-NEXT:    vcvtsd2ss %xmm0, %xmm0, %xmm0
+; F16C-NEXT:    vcvtps2ph $4, %xmm0, %xmm0
+; F16C-NEXT:    vmovd %xmm0, %eax
+; F16C-NEXT:    vpinsrw $0, %eax, %xmm0, %xmm0
+; F16C-NEXT:    retq
+;
+; AVX-LABEL: test3:
+; AVX:       # %bb.0:
+; AVX-NEXT:    pushq %rax
+; AVX-NEXT:    vcvtsd2ss %xmm0, %xmm0, %xmm0
+; AVX-NEXT:    callq __truncsfhf2@PLT
+; AVX-NEXT:    popq %rax
+; AVX-NEXT:    retq
+  %1 = fptrunc double %d to float
+  %2 = fptrunc float %1 to half
+  ret half %2
+}
+
+define <2 x half> @test3_vector_fast(<2 x double> %d) #0 {
+; F16C-LABEL: test3_vector_fast:
+; F16C:       # %bb.0:
+; F16C-NEXT:    vcvtpd2ps %xmm0, %xmm0
+; F16C-NEXT:    vcvtps2ph $4, %xmm0, %xmm0
+; F16C-NEXT:    retq
+;
+; AVX-LABEL: test3_vector_fast:
+; AVX:       # %bb.0:
+; AVX-NEXT:    subq $40, %rsp
+; AVX-NEXT:    vcvtpd2ps %xmm0, %xmm0
+; AVX-NEXT:    vmovapd %xmm0, {{[-0-9]+}}(%r{{[sb]}}p) # 16-byte Spill
+; AVX-NEXT:    vmovshdup {{.*#+}} xmm0 = xmm0[1,1,3,3]
+; AVX-NEXT:    callq __truncsfhf2@PLT
+; AVX-NEXT:    vmovaps %xmm0, (%rsp) # 16-byte Spill
+; AVX-NEXT:    vmovdqa {{[-0-9]+}}(%r{{[sb]}}p), %xmm0 # 16-byte Reload
+; AVX-NEXT:    callq __truncsfhf2@PLT
+; AVX-NEXT:    vpunpcklwd (%rsp), %xmm0, %xmm0 # 16-byte Folded Reload
+; AVX-NEXT:    # xmm0 = xmm0[0],mem[0],xmm0[1],mem[1],xmm0[2],mem[2],xmm0[3],mem[3]
+; AVX-NEXT:    addq $40, %rsp
+; AVX-NEXT:    retq
+  %1 = fptrunc <2 x double> %d to <2 x float>
+  %2 = fptrunc <2 x float> %1 to <2 x half>
+  ret <2 x half> %2
+}
+
+define <2 x half> @test3_vector(<2 x double> %d) #1 {
+; F16C-LABEL: test3_vector:
+; F16C:       # %bb.0:
+; F16C-NEXT:    vcvtpd2ps %xmm0, %xmm0
+; F16C-NEXT:    vcvtps2ph $4, %xmm0, %xmm0
+; F16C-NEXT:    retq
+;
+; AVX-LABEL: test3_vector:
+; AVX:       # %bb.0:
+; AVX-NEXT:    subq $40, %rsp
+; AVX-NEXT:    vcvtpd2ps %xmm0, %xmm0
+; AVX-NEXT:    vmovapd %xmm0, {{[-0-9]+}}(%r{{[sb]}}p) # 16-byte Spill
+; AVX-NEXT:    vmovshdup {{.*#+}} xmm0 = xmm0[1,1,3,3]
+; AVX-NEXT:    callq __truncsfhf2@PLT
+; AVX-NEXT:    vmovaps %xmm0, (%rsp) # 16-byte Spill
+; AVX-NEXT:    vmovdqa {{[-0-9]+}}(%r{{[sb]}}p), %xmm0 # 16-byte Reload
+; AVX-NEXT:    callq __truncsfhf2@PLT
+; AVX-NEXT:    vpunpcklwd (%rsp), %xmm0, %xmm0 # 16-byte Folded Reload
+; AVX-NEXT:    # xmm0 = xmm0[0],mem[0],xmm0[1],mem[1],xmm0[2],mem[2],xmm0[3],mem[3]
+; AVX-NEXT:    addq $40, %rsp
+; AVX-NEXT:    retq
+  %1 = fptrunc <2 x double> %d to <2 x float>
+  %2 = fptrunc <2 x float> %1 to <2 x half>
+  ret <2 x half> %2
+}
+
+; The 'test4*' versions convert:
+;   x86_fp80 -> float -> half
+; Like the 'test3*' versions, we don't want to fold, even when
+; 'unsafe-fp-math' is true.
+define half @test4_fast(x86_fp80 %d) #0 {
+; F16C-LABEL: test4_fast:
+; F16C:       # %bb.0:
+; F16C-NEXT:    fldt {{[0-9]+}}(%rsp)
+; F16C-NEXT:    fstps -{{[0-9]+}}(%rsp)
+; F16C-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; F16C-NEXT:    vcvtps2ph $4, %xmm0, %xmm0
+; F16C-NEXT:    vmovd %xmm0, %eax
+; F16C-NEXT:    vpinsrw $0, %eax, %xmm0, %xmm0
+; F16C-NEXT:    retq
+;
+; AVX-LABEL: test4_fast:
+; AVX:       # %bb.0:
+; AVX-NEXT:    pushq %rax
+; AVX-NEXT:    fldt {{[0-9]+}}(%rsp)
+; AVX-NEXT:    fstps {{[0-9]+}}(%rsp)
+; AVX-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; AVX-NEXT:    callq __truncsfhf2@PLT
+; AVX-NEXT:    popq %rax
+; AVX-NEXT:    retq
+  %1 = fptrunc x86_fp80 %d to float
+  %2 = fptrunc float %1 to half
+  ret half %2
+}
+
+define half @test4(x86_fp80 %d) #1 {
+; F16C-LABEL: test4:
+; F16C:       # %bb.0:
+; F16C-NEXT:    fldt {{[0-9]+}}(%rsp)
+; F16C-NEXT:    fstps -{{[0-9]+}}(%rsp)
+; F16C-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; F16C-NEXT:    vcvtps2ph $4, %xmm0, %xmm0
+; F16C-NEXT:    vmovd %xmm0, %eax
+; F16C-NEXT:    vpinsrw $0, %eax, %xmm0, %xmm0
+; F16C-NEXT:    retq
+;
+; AVX-LABEL: test4:
+; AVX:       # %bb.0:
+; AVX-NEXT:    pushq %rax
+; AVX-NEXT:    fldt {{[0-9]+}}(%rsp)
+; AVX-NEXT:    fstps {{[0-9]+}}(%rsp)
+; AVX-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; AVX-NEXT:    callq __truncsfhf2@PLT
+; AVX-NEXT:    popq %rax
+; AVX-NEXT:    retq
+  %1 = fptrunc x86_fp80 %d to float
+  %2 = fptrunc float %1 to half
+  ret half %2
+}
+
+define <2 x half> @test4_vector_fast(<2 x x86_fp80> %d) #0 {
+; F16C-LABEL: test4_vector_fast:
+; F16C:       # %bb.0:
+; F16C-NEXT:    fldt {{[0-9]+}}(%rsp)
+; F16C-NEXT:    fldt {{[0-9]+}}(%rsp)
+; F16C-NEXT:    fstps -{{[0-9]+}}(%rsp)
+; F16C-NEXT:    fstps -{{[0-9]+}}(%rsp)
+; F16C-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; F16C-NEXT:    vinsertps {{.*#+}} xmm0 = xmm0[0],mem[0],zero,zero
+; F16C-NEXT:    vcvtps2ph $4, %xmm0, %xmm0
+; F16C-NEXT:    retq
+;
+; AVX-LABEL: test4_vector_fast:
+; AVX:       # %bb.0:
+; AVX-NEXT:    subq $40, %rsp
+; AVX-NEXT:    fldt {{[0-9]+}}(%rsp)
+; AVX-NEXT:    fldt {{[0-9]+}}(%rsp)
+; AVX-NEXT:    fstps {{[0-9]+}}(%rsp)
+; AVX-NEXT:    fstps {{[0-9]+}}(%rsp)
+; AVX-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; AVX-NEXT:    vmovss %xmm0, {{[-0-9]+}}(%r{{[sb]}}p) # 4-byte Spill
+; AVX-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; AVX-NEXT:    callq __truncsfhf2@PLT
+; AVX-NEXT:    vmovaps %xmm0, {{[-0-9]+}}(%r{{[sb]}}p) # 16-byte Spill
+; AVX-NEXT:    vmovd {{[-0-9]+}}(%r{{[sb]}}p), %xmm0 # 4-byte Folded Reload
+; AVX-NEXT:    # xmm0 = mem[0],zero,zero,zero
+; AVX-NEXT:    callq __truncsfhf2@PLT
+; AVX-NEXT:    vmovdqa {{[-0-9]+}}(%r{{[sb]}}p), %xmm1 # 16-byte Reload
+; AVX-NEXT:    vpunpcklwd {{.*#+}} xmm0 = xmm1[0],xmm0[0],xmm1[1],xmm0[1],xmm1[2],xmm0[2],xmm1[3],xmm0[3]
+; AVX-NEXT:    addq $40, %rsp
+; AVX-NEXT:    retq
+  %1 = fptrunc <2 x x86_fp80> %d to <2 x float>
+  %2 = fptrunc <2 x float> %1 to <2 x half>
+  ret <2 x half> %2
+}
+
+define <2 x half> @test4_vector(<2 x x86_fp80> %d) #1 {
+; F16C-LABEL: test4_vector:
+; F16C:       # %bb.0:
+; F16C-NEXT:    fldt {{[0-9]+}}(%rsp)
+; F16C-NEXT:    fldt {{[0-9]+}}(%rsp)
+; F16C-NEXT:    fstps -{{[0-9]+}}(%rsp)
+; F16C-NEXT:    fstps -{{[0-9]+}}(%rsp)
+; F16C-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; F16C-NEXT:    vinsertps {{.*#+}} xmm0 = xmm0[0],mem[0],zero,zero
+; F16C-NEXT:    vcvtps2ph $4, %xmm0, %xmm0
+; F16C-NEXT:    retq
+;
+; AVX-LABEL: test4_vector:
+; AVX:       # %bb.0:
+; AVX-NEXT:    subq $40, %rsp
+; AVX-NEXT:    fldt {{[0-9]+}}(%rsp)
+; AVX-NEXT:    fldt {{[0-9]+}}(%rsp)
+; AVX-NEXT:    fstps {{[0-9]+}}(%rsp)
+; AVX-NEXT:    fstps {{[0-9]+}}(%rsp)
+; AVX-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; AVX-NEXT:    vmovss %xmm0, {{[-0-9]+}}(%r{{[sb]}}p) # 4-byte Spill
+; AVX-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
+; AVX-NEXT:    callq __truncsfhf2@PLT
+; AVX-NEXT:    vmovaps %xmm0, {{[-0-9]+}}(%r{{[sb]}}p) # 16-byte Spill
+; AVX-NEXT:    vmovd {{[-0-9]+}}(%r{{[sb]}}p), %xmm0 # 4-byte Folded Reload
+; AVX-NEXT:    # xmm0 = mem[0],zero,zero,zero
+; AVX-NEXT:    callq __truncsfhf2@PLT
+; AVX-NEXT:    vmovdqa {{[-0-9]+}}(%r{{[sb]}}p), %xmm1 # 16-byte Reload
+; AVX-NEXT:    vpunpcklwd {{.*#+}} xmm0 = xmm1[0],xmm0[0],xmm1[1],xmm0[1],xmm1[2],xmm0[2],xmm1[3],xmm0[3]
+; AVX-NEXT:    addq $40, %rsp
+; AVX-NEXT:    retq
+  %1 = fptrunc <2 x x86_fp80> %d to <2 x float>
+  %2 = fptrunc <2 x float> %1 to <2 x half>
+  ret <2 x half> %2
+}
+
 declare i16 @llvm.convert.to.fp16.f64(double)
 declare i16 @llvm.convert.to.fp16.f80(x86_fp80)
 
 attributes #0 = { nounwind readnone "unsafe-fp-math"="true" "use-soft-float"="false" }
 attributes #1 = { nounwind readnone "unsafe-fp-math"="false" "use-soft-float"="false" }