AMDGPU/SI: Re-implement the lowering for 32-bit floating point division

[cfang]

Summary

Expand fdiv32 instruction to generate the desired precision following IEEE spec.

NOTE: we may still need to modify fdiv.ll test to check the exact code sequence! This implementation has mainly been done by Wei!

[arsenm]

Can you reupload the diff with context. Is this fixing the lowering of just frem?

[cfang]

New diff posted. This actually implements the lowering of 32-bit floating point division: SITargetLowering::LowerFDIV32.
It affects frem because frem is using fdiv.

[arsenm]

The fdiv test must be modified.

We should not simply replace the division lowering this way. This should be controlled by precision options, so the existing lowering should still be able to work

[cfang]

Update based on Matt's comments:

1. Keep the original fpdiv32 expansion under fast_math;
2. update the LIT test for fpdiv.ll

[cfang]

update a typo. && Ping

[arsenm]

test/CodeGen/AMDGPU/fdiv.ll

83–88

Can you add a test where the fast math flag is missing from the interaction, but globally enabled?

129–134

A lot of these are broken with _. Also note that repeating the same instruction multiple times with -DAG does not behave as expected

[cfang]

Add a test with function attribute "unsafe-fp-math=true";
Correct a typo (SI-DAG instead of SI_DAG)

Thanks;

[arsenm]

A version of this was committed a while ago

lib/Target/AMDGPU/SIISelLowering.cpp

   // selection error for now rather than do something incorrect.
   if (Subtarget->hasFP32Denormals())
     return SDValue();
 
   SDLoc SL(Op);
   SDValue LHS = Op.getOperand(0);
   SDValue RHS = Op.getOperand(1);
 
-  SDValue r1 = DAG.getNode(ISD::FABS, SL, MVT::f32, RHS);
+  // Generate the efficient but not precise fpdiv32 if fast_math is ON
+  const SDNodeFlags *Flags = Op.getNode()->getFlags();
+  if (Flags->hasUnsafeAlgebra()) {
+    SDValue r1 = DAG.getNode(ISD::FABS, SL, MVT::f32, RHS);
 
     const APFloat K0Val(BitsToFloat(0x6f800000));
     const SDValue K0 = DAG.getConstantFP(K0Val, SL, MVT::f32);
 
     const APFloat K1Val(BitsToFloat(0x2f800000));
     const SDValue K1 = DAG.getConstantFP(K1Val, SL, MVT::f32);
 
+    const SDValue One = DAG.getConstantFP(1.0, SL, MVT::f32);
+
+    EVT SetCCVT =
+        getSetCCResultType(DAG.getDataLayout(), *DAG.getContext(), MVT::f32);
+
+    SDValue r2 = DAG.getSetCC(SL, SetCCVT, r1, K0, ISD::SETOGT);
+
+    SDValue r3 = DAG.getNode(ISD::SELECT, SL, MVT::f32, r2, K1, One);
+
+    // TODO: Should this propagate fast-math-flags?
+
+    r1 = DAG.getNode(ISD::FMUL, SL, MVT::f32, RHS, r3);
+
+    SDValue r0 = DAG.getNode(AMDGPUISD::RCP, SL, MVT::f32, r1);
+
+    SDValue Mul = DAG.getNode(ISD::FMUL, SL, MVT::f32, LHS, r0);
+
+    return DAG.getNode(ISD::FMUL, SL, MVT::f32, r3, Mul);
+  }
+
+  // Generates more precise fpdiv32.
   const SDValue One = DAG.getConstantFP(1.0, SL, MVT::f32);
 
-  EVT SetCCVT =
-      getSetCCResultType(DAG.getDataLayout(), *DAG.getContext(), MVT::f32);
+  SDVTList ScaleVT = DAG.getVTList(MVT::f32, MVT::i1);
 
-  SDValue r2 = DAG.getSetCC(SL, SetCCVT, r1, K0, ISD::SETOGT);
+  SDValue d_scaled = DAG.getNode(AMDGPUISD::DIV_SCALE, SL, ScaleVT, RHS, RHS, LHS);
+  SDValue n_scaled = DAG.getNode(AMDGPUISD::DIV_SCALE, SL, ScaleVT, LHS, RHS, LHS);
 
-  SDValue r3 = DAG.getNode(ISD::SELECT, SL, MVT::f32, r2, K1, One);
+  SDValue approx_rcp = DAG.getNode(AMDGPUISD::RCP, SL, MVT::f32, d_scaled);
 
-  // TODO: Should this propagate fast-math-flags?
+  SDValue NegDivScale0 = DAG.getNode(ISD::FNEG, SL, MVT::f32, d_scaled);
+
+  SDValue Fma0 = DAG.getNode(ISD::FMA, SL, MVT::f32, NegDivScale0, approx_rcp, One);
+  SDValue Fma1 = DAG.getNode(ISD::FMA, SL, MVT::f32, Fma0, approx_rcp, approx_rcp);
 
-  r1 = DAG.getNode(ISD::FMUL, SL, MVT::f32, RHS, r3);
+  SDValue Mul = DAG.getNode(ISD::FMUL, SL, MVT::f32, n_scaled, Fma1);
 
-  SDValue r0 = DAG.getNode(AMDGPUISD::RCP, SL, MVT::f32, r1);
+  SDValue Fma2 = DAG.getNode(ISD::FMA, SL, MVT::f32, NegDivScale0, Mul, n_scaled);
+  SDValue Fma3 = DAG.getNode(ISD::FMA, SL, MVT::f32, Fma2, Fma1, Mul);
+  SDValue Fma4 = DAG.getNode(ISD::FMA, SL, MVT::f32, NegDivScale0, Fma3, n_scaled);
 
-  SDValue Mul = DAG.getNode(ISD::FMUL, SL, MVT::f32, LHS, r0);
+  SDValue Scale = n_scaled.getValue(1);
+  SDValue Fmas = DAG.getNode(AMDGPUISD::DIV_FMAS, SL, MVT::f32, Fma4, Fma1, Fma3, Scale);
 
-  return DAG.getNode(ISD::FMUL, SL, MVT::f32, r3, Mul);
+  return DAG.getNode(AMDGPUISD::DIV_FIXUP, SL, MVT::f32, Fmas, RHS, LHS);
 }
 
 SDValue SITargetLowering::LowerFDIV64(SDValue Op, SelectionDAG &DAG) const {
   if (DAG.getTarget().Options.UnsafeFPMath)
     return LowerFastFDIV(Op, DAG);
 
   SDLoc SL(Op);
   SDValue X = Op.getOperand(0);

test/CodeGen/AMDGPU/fdiv.ll

 ; RUN: llc -march=r600 -mcpu=redwood < %s | FileCheck -check-prefix=R600 %s
 ; RUN: llc -march=amdgcn -mcpu=SI -verify-machineinstrs < %s | FileCheck -check-prefix=SI %s
 ; RUN: llc -march=amdgcn -mcpu=tonga -verify-machineinstrs < %s | FileCheck -check-prefix=SI %s
 
 ; These tests check that fdiv is expanded correctly and also test that the
 ; scheduler is scheduling the RECIP_IEEE and MUL_IEEE instructions in separate
 ; instruction groups.
 
-; FUNC-LABEL: {{^}}fdiv_f32:
+; FUNC-LABEL: {{^}}fdiv_f32_fast_math:
 ; R600-DAG: RECIP_IEEE * T{{[0-9]+\.[XYZW]}}, KC0[3].Z
 ; R600-DAG: RECIP_IEEE * T{{[0-9]+\.[XYZW]}}, KC0[3].Y
 ; R600-DAG: MUL_IEEE {{\** *}}T{{[0-9]+\.[XYZW]}}, KC0[3].X, PS
 ; R600-DAG: MUL_IEEE {{\** *}}T{{[0-9]+\.[XYZW]}}, KC0[2].W, PS
 
 ; SI-DAG: v_rcp_f32
 ; SI-DAG: v_mul_f32
-define void @fdiv_f32(float addrspace(1)* %out, float %a, float %b) {
+define void @fdiv_f32_fast_math(float addrspace(1)* %out, float %a, float %b) {
 entry:
-  %0 = fdiv float %a, %b
+  %0 = fdiv fast float %a, %b
   store float %0, float addrspace(1)* %out
   ret void
 }
 
 
-
-; FUNC-LABEL: {{^}}fdiv_v2f32:
+; FUNC-LABEL: {{^}}fdiv_v2f32_fast_math:
 ; R600-DAG: RECIP_IEEE * T{{[0-9]+\.[XYZW]}}, KC0[3].Z
 ; R600-DAG: RECIP_IEEE * T{{[0-9]+\.[XYZW]}}, KC0[3].Y
 ; R600-DAG: MUL_IEEE {{\** *}}T{{[0-9]+\.[XYZW]}}, KC0[3].X, PS
 ; R600-DAG: MUL_IEEE {{\** *}}T{{[0-9]+\.[XYZW]}}, KC0[2].W, PS
 
 ; SI-DAG: v_rcp_f32
 ; SI-DAG: v_mul_f32
 ; SI-DAG: v_rcp_f32
 ; SI-DAG: v_mul_f32
-define void @fdiv_v2f32(<2 x float> addrspace(1)* %out, <2 x float> %a, <2 x float> %b) {
+define void @fdiv_v2f32_fast_math(<2 x float> addrspace(1)* %out, <2 x float> %a, <2 x float> %b) {
 entry:
-  %0 = fdiv <2 x float> %a, %b
+  %0 = fdiv fast <2 x float> %a, %b
   store <2 x float> %0, <2 x float> addrspace(1)* %out
   ret void
 }
 
-; FUNC-LABEL: {{^}}fdiv_v4f32:
+; FUNC-LABEL: {{^}}fdiv_v4f32_fast_math:
 ; R600-DAG: RECIP_IEEE * T{{[0-9]+\.[XYZW], T[0-9]+\.[XYZW]}}
 ; R600-DAG: RECIP_IEEE * T{{[0-9]+\.[XYZW], T[0-9]+\.[XYZW]}}
 ; R600-DAG: RECIP_IEEE * T{{[0-9]+\.[XYZW], T[0-9]+\.[XYZW]}}
 ; R600-DAG: RECIP_IEEE * T{{[0-9]+\.[XYZW], T[0-9]+\.[XYZW]}}
 ; R600-DAG: MUL_IEEE {{\** *}}T{{[0-9]+\.[XYZW], T[0-9]+\.[XYZW]}}, PS
 ; R600-DAG: MUL_IEEE {{\** *}}T{{[0-9]+\.[XYZW], T[0-9]+\.[XYZW]}}, PS
 ; R600-DAG: MUL_IEEE {{\** *}}T{{[0-9]+\.[XYZW], T[0-9]+\.[XYZW]}}, PS
 ; R600-DAG: MUL_IEEE {{\** *}}T{{[0-9]+\.[XYZW], T[0-9]+\.[XYZW]}}, PS
 
 ; SI-DAG: v_rcp_f32
 ; SI-DAG: v_mul_f32
 ; SI-DAG: v_rcp_f32
 ; SI-DAG: v_mul_f32
 ; SI-DAG: v_rcp_f32
 ; SI-DAG: v_mul_f32
 ; SI-DAG: v_rcp_f32
 ; SI-DAG: v_mul_f32
+define void @fdiv_v4f32_fast_math(<4 x float> addrspace(1)* %out, <4 x float> addrspace(1)* %in) {
+  %b_ptr = getelementptr <4 x float>, <4 x float> addrspace(1)* %in, i32 1
+  %a = load <4 x float>, <4 x float> addrspace(1) * %in
+  %b = load <4 x float>, <4 x float> addrspace(1) * %b_ptr
+  %result = fdiv fast <4 x float> %a, %b
+  store <4 x float> %result, <4 x float> addrspace(1)* %out
+  ret void
+}
+
+
+; FUNC-LABEL: {{^}}fdiv_f32:
+
+; SI-DAG: v_div_scale_f32
+; SI-DAG: v_rcp_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_mul_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_div_fixup_f32
+define void @fdiv_f32(float addrspace(1)* %out, float %a, float %b) {
+entry:
+  %0 = fdiv float %a, %b
+  store float %0, float addrspace(1)* %out
+  ret void
+}
+
+
+
+; FUNC-LABEL: {{^}}fdiv_v2f32:
+
+; SI-DAG: v_div_scale_f32
+; SI-DAG: v_rcp_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_mul_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_div_fixup_f32
+; SI-DAG: v_div_scale_f32
+; SI-DAG: v_rcp_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_mul_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_div_fixup_f32
+define void @fdiv_v2f32(<2 x float> addrspace(1)* %out, <2 x float> %a, <2 x float> %b) {
+entry:
+  %0 = fdiv <2 x float> %a, %b
+  store <2 x float> %0, <2 x float> addrspace(1)* %out
+  ret void
+}
+
+
+; FUNC-LABEL: {{^}}fdiv_v4f32:
+
+; SI-DAG: v_div_scale_f32
+; SI-DAG: v_rcp_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_mul_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_div_fixup_f32
+; SI-DAG: v_div_scale_f32
+; SI-DAG: v_rcp_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_mul_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_div_fixup_f32
+; SI-DAG: v_div_scale_f32
+; SI-DAG: v_rcp_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_mul_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_div_fixup_f32
+; SI-DAG: v_div_scale_f32
+; SI-DAG: v_rcp_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_mul_f32
+; SI-DAG: v_fma_f32
+; SI-DAG: v_div_fixup_f32
 define void @fdiv_v4f32(<4 x float> addrspace(1)* %out, <4 x float> addrspace(1)* %in) {
   %b_ptr = getelementptr <4 x float>, <4 x float> addrspace(1)* %in, i32 1
   %a = load <4 x float>, <4 x float> addrspace(1) * %in
   %b = load <4 x float>, <4 x float> addrspace(1) * %b_ptr
   %result = fdiv <4 x float> %a, %b
   store <4 x float> %result, <4 x float> addrspace(1)* %out
   ret void
 }
+
+
+; FUNC-LABEL: {{^}}fdiv_f32_unsafe_math_attr:
+
+; SI-DAG: v_rcp_f32
+; SI-DAG: v_mul_f32
+define void @fdiv_f32_unsafe_math_attr(float addrspace(1)* %out, float %a, float %b) #0  {
+entry:
+  %0 = fdiv float %a, %b
+  store float %0, float addrspace(1)* %out
+  ret void
+}
+
+attributes #0 = { "unsafe-fp-math"="true" }

test/CodeGen/AMDGPU/frem.ll

 ; RUN: llc -march=amdgcn -mcpu=SI -enable-misched < %s | FileCheck -check-prefix=SI -check-prefix=GCN -check-prefix=FUNC %s
 ; RUN: llc -march=amdgcn -mcpu=bonaire -enable-misched < %s | FileCheck -check-prefix=CI -check-prefix=GCN -check-prefix=FUNC %s
 ; RUN: llc -march=amdgcn -mcpu=tonga -enable-misched < %s | FileCheck -check-prefix=CI -check-prefix=GCN -check-prefix=FUNC %s
 
 ; FUNC-LABEL: {{^}}frem_f32:
 ; GCN-DAG: buffer_load_dword [[X:v[0-9]+]], {{.*$}}
 ; GCN-DAG: buffer_load_dword [[Y:v[0-9]+]], {{.*}} offset:16
-; GCN-DAG: v_cmp
-; GCN-DAG: v_mul_f32
+; GCN: v_div_scale_f32
 ; GCN: v_rcp_f32_e32
+; GCN: v_fma_f32
 ; GCN: v_mul_f32_e32
-; GCN: v_mul_f32_e32
+; GCN: v_div_fmas_f32
+; GCN: v_div_fixup_f32
 ; GCN: v_trunc_f32_e32
 ; GCN: v_mad_f32
 ; GCN: s_endpgm
 define void @frem_f32(float addrspace(1)* %out, float addrspace(1)* %in1,
                       float addrspace(1)* %in2) #0 {
    %gep2 = getelementptr float, float addrspace(1)* %in2, i32 4
    %r0 = load float, float addrspace(1)* %in1, align 4
    %r1 = load float, float addrspace(1)* %gep2, align 4