AMDGPU : Replace FMAD with FMA when denormals are enabled.

[wdng]

Summary

AMDGPU : Replace FMAD with FMA when denormals are enabled.

[arsenm]

You should not be attempting to lower fmad. It is not the same as FMA. You should be creating a new FMAD_FTZ node for use in the one specific case you are trying to fix, which will only be used if denormals are disabled.

[wdng]

Address code reviews.

[b-sumner]

This comment was removed by b-sumner.

[arsenm]

lib/Target/AMDGPU/AMDGPUISelLowering.cpp

1296–1300

You only need to select between the opcode. You don't need 2 getNode calls

lib/Target/AMDGPU/AMDGPUInstrInfo.td

198

Extra newline

lib/Target/AMDGPU/SIISelLowering.cpp

365 ↗

(On Diff #88572)

Not sure why this appears in the diff

lib/Target/AMDGPU/SIInstructions.td

500–505

You should be using the exact fmad pattern above. You should refactor the class to take the input node

test/CodeGen/AMDGPU/udiv.ll

7–8

You should only need one new run line, since the default is already tested. You should change the existing VI runline to explicitly set off. The r600 run line should also be last

193–195

You don't need both functions since you are using the global -mattr to do this. These also should have the same result with and without denormals

195

You should minimize this list

[wdng]

As complex pattern causes pattern cannot be selected. So no complex pattern used for FMAD_FTZ pattern definition. Also the generated ISAs is different as fneg is being folded, so we have v_mac_f32 and v_mad_f32 respectively when denormals are enabled and disabled.

[b-sumner]

v_mac_f32 always flushes subnormal inputs just like v_mad_f32

[wdng]

Change V_MAC_f32 to V_MAD_F32 to emit the same result with and without denormals.

[arsenm]

lib/Target/AMDGPU/AMDGPUISelLowering.cpp

1297

The casts are unnecessary

lib/Target/AMDGPU/SIInstructions.td

505

This won't fold the neg source modifier, so will still make code worse. There should be a 3-operand class with source modifiers you should use rather than calling it FMAC. This also can be a class, a multi class isn't necessary

test/CodeGen/AMDGPU/udiv.ll

166

Should check the neg source modifier is used

[wdng]

lib/Target/AMDGPU/AMDGPUISelLowering.cpp

1297

If cast is removed, there will be a "warning: enumeral mismatch in conditional expression: ‘llvm::AMDGPUISD::NodeType’ vs ‘llvm::ISD::NodeType’", shall we still remove it?

[wdng]

Address code reviews.

[arsenm]

lib/Target/AMDGPU/SIInstructions.td

511

For now you might want to just stick emitting v_mad_f32 to not deviate from the denormal disabled output. Selecting v_mac_ with src0/src1 modifiers should be a separate patch

[wdng]

Address code review feedback.

[arsenm]

lib/Target/AMDGPU/AMDGPUISelLowering.h

269

Needs a comment explaining what it is for

lib/Target/AMDGPU/SIInstructions.td

505

Extra space after VOP3Mods

[wdng]

Address code review feedback.

[arsenm]

lib/Target/AMDGPU/AMDGPUISelLowering.h

269

This is the opposite. It is for emitting mad when f32 denormals are enabled because mac/mad always flush

[wdng]

Update comments.

[wdng]

ping.

[arsenm]

lib/Target/AMDGPU/AMDGPUISelLowering.h

269

should refer to it as ISD::FMAD here

270

It's an illegal operation, not an illegal type

[wdng]

Updated, thanks!

[wdng]

ping.

[arsenm]

LGTM

lib/Target/AMDGPU/AMDGPUISelLowering.h

enum NodeType : unsigned {
   UMIN3,
   FMED3,
   SMED3,
   UMED3,
   URECIP,
   DIV_SCALE,
   DIV_FMAS,
   DIV_FIXUP,
+  FMAD_FTZ, // For f32, handles denormals.
   TRIG_PREOP, // 1 ULP max error for f64
 
   // RCP, RSQ - For f32, 1 ULP max error, no denormal handling.
   //            For f64, max error 2^29 ULP, handles denormals.
   RCP,
   RSQ,
   RCP_LEGACY,
   RSQ_LEGACY,
   FMUL_LEGACY,

lib/Target/AMDGPU/AMDGPUISelLowering.cpp

SDValue AMDGPUTargetLowering::LowerDIVREM24(SDValue Op, SelectionDAG &DAG,
 
   // fq = trunc(fq);
   fq = DAG.getNode(ISD::FTRUNC, DL, FltVT, fq);
 
   // float fqneg = -fq;
   SDValue fqneg = DAG.getNode(ISD::FNEG, DL, FltVT, fq);
 
   // float fr = mad(fqneg, fb, fa);
-  SDValue fr = DAG.getNode(ISD::FMAD, DL, FltVT, fqneg, fb, fa);
+  unsigned OpCode = Subtarget->hasFP32Denormals() ? 
+                    (unsigned)AMDGPUISD::FMAD_FTZ : 
+                    (unsigned)ISD::FMAD;
+  SDValue fr = DAG.getNode(OpCode, DL, FltVT, fqneg, fb, fa);
 
   // int iq = (int)fq;
   SDValue iq = DAG.getNode(ToInt, DL, IntVT, fq);
 
   // fr = fabs(fr);
   fr = DAG.getNode(ISD::FABS, DL, FltVT, fr);
 
   // fb = fabs(fb);
   fb = DAG.getNode(ISD::FABS, DL, FltVT, fb);
const char* AMDGPUTargetLowering::getTargetNodeName(unsigned Opcode) const {
   NODE_NAME_CASE(UMIN3)
   NODE_NAME_CASE(FMED3)
   NODE_NAME_CASE(SMED3)
   NODE_NAME_CASE(UMED3)
   NODE_NAME_CASE(URECIP)
   NODE_NAME_CASE(DIV_SCALE)
   NODE_NAME_CASE(DIV_FMAS)
   NODE_NAME_CASE(DIV_FIXUP)
+  NODE_NAME_CASE(FMAD_FTZ)
   NODE_NAME_CASE(TRIG_PREOP)
   NODE_NAME_CASE(RCP)
   NODE_NAME_CASE(RSQ)
   NODE_NAME_CASE(RCP_LEGACY)
   NODE_NAME_CASE(RSQ_LEGACY)
   NODE_NAME_CASE(FMUL_LEGACY)
   NODE_NAME_CASE(RSQ_CLAMP)
   NODE_NAME_CASE(LDEXP)

lib/Target/AMDGPU/AMDGPUInstrInfo.td

 //  src1 = Denominator, src2 = Numerator).
 def AMDGPUdiv_fmas : SDNode<"AMDGPUISD::DIV_FMAS", AMDGPUFmasOp>;
 
 // Single or double precision division fixup.
 // Special case divide fixup and flags(src0 = Quotient, src1 =
 // Denominator, src2 = Numerator).
 def AMDGPUdiv_fixup : SDNode<"AMDGPUISD::DIV_FIXUP", SDTFPTernaryOp>;
 
+def AMDGPUfmad_ftz : SDNode<"AMDGPUISD::FMAD_FTZ", SDTFPTernaryOp>;
+
 // Look Up 2.0 / pi src0 with segment select src1[4:0]
 def AMDGPUtrig_preop : SDNode<"AMDGPUISD::TRIG_PREOP", AMDGPUTrigPreOp>;
 
 def AMDGPUregister_load : SDNode<"AMDGPUISD::REGISTER_LOAD",
                           SDTypeProfile<1, 2, [SDTCisPtrTy<1>, SDTCisInt<2>]>,
                           [SDNPHasChain, SDNPMayLoad]>;
 
 def AMDGPUregister_store : SDNode<"AMDGPUISD::REGISTER_STORE",

lib/Target/AMDGPU/SIInstructions.td

def : Pat <
     (vt (fmad (VOP3NoMods0 vt:$src0, i32:$src0_modifiers, i1:$clamp, i32:$omod),
               (VOP3NoMods  vt:$src1, i32:$src1_modifiers),
               (VOP3NoMods  vt:$src2, i32:$src2_modifiers))),
     (inst $src0_modifiers, $src0, $src1_modifiers, $src1,
           $src2_modifiers, $src2, $clamp, $omod)
   >;
 }
 
 defm : FMADPat <f16, V_MAC_F16_e64>;
 defm : FMADPat <f32, V_MAC_F32_e64>;
 
+class FMADModsPat<Instruction inst, SDPatternOperator mad_opr> : Pat<
+  (f32 (mad_opr (VOP3Mods f32:$src0, i32:$src0_mod),
+  (VOP3Mods f32:$src1, i32:$src1_mod),
+  (VOP3Mods f32:$src2, i32:$src2_mod))),
+  (inst $src0_mod, $src0, $src1_mod, $src1,
+  $src2_mod, $src2, DSTCLAMP.NONE, DSTOMOD.NONE)
+>;
+
+def : FMADModsPat<V_MAD_F32, AMDGPUfmad_ftz>;
+
 multiclass SelectPat <ValueType vt, Instruction inst> {
   def : Pat <
     (vt (select i1:$src0, vt:$src1, vt:$src2)),
     (inst $src2, $src1, $src0)
   >;
 }
 
 defm : SelectPat <i16, V_CNDMASK_B32_e64>;

test/CodeGen/AMDGPU/udiv.ll

 ; RUN: llc -march=amdgcn -mcpu=verde -verify-machineinstrs < %s | FileCheck -check-prefix=SI -check-prefix=FUNC %s
-; RUN: llc -march=amdgcn -mcpu=tonga -mattr=-flat-for-global -verify-machineinstrs < %s | FileCheck -check-prefix=SI -check-prefix=FUNC %s
+; RUN: llc -march=amdgcn -mcpu=tonga -mattr=-flat-for-global -verify-machineinstrs -mattr=-fp32-denormals < %s | FileCheck -check-prefix=SI -check-prefix=FUNC -check-prefix=VI %s
+
+; RUN: llc -mtriple=amdgcn--amdhsa -mcpu=fiji -mattr=+fp32-denormals < %s | FileCheck -check-prefix=GCN -check-prefix=VI %s
+
 ; RUN: llc -march=r600 -mcpu=redwood < %s | FileCheck -check-prefix=EG -check-prefix=FUNC %s
 
 ; FUNC-LABEL: {{^}}udiv_i32:
 ; EG-NOT: SETGE_INT
 ; EG: CF_END
 define void @udiv_i32(i32 addrspace(1)* %out, i32 addrspace(1)* %in) {
   %b_ptr = getelementptr i32, i32 addrspace(1)* %in, i32 1
   %a = load i32, i32 addrspace(1) * %in
   %b = load i32, i32 addrspace(1) * %b_ptr
   %result = udiv i32 %a, %b
   store i32 %result, i32 addrspace(1)* %out

 ; SI: v_mul_hi_u32
 
 define void @scalarize_mulhu_4xi32(<4 x i32> addrspace(1)* nocapture readonly %in, <4 x i32> addrspace(1)* nocapture %out) {
   %1 = load <4 x i32>, <4 x i32> addrspace(1)* %in, align 16
   %2 = udiv <4 x i32> %1, <i32 53668, i32 53668, i32 53668, i32 53668>
   store <4 x i32> %2, <4 x i32> addrspace(1)* %out, align 16
   ret void
 }
+
+; GCN-LABEL: {{^}}fdiv_test_denormals
+; VI: v_mad_f32 v{{[0-9]+}}, -v{{[0-9]+}}, v{{[0-9]+}}, v{{[0-9]+}}
+define amdgpu_kernel void @fdiv_test_denormals(i8 addrspace(1)* nocapture readonly %arg) {
+bb:
+  %tmp = load i8, i8 addrspace(1)* null, align 1
+  %tmp1 = sext i8 %tmp to i32
+  %tmp2 = getelementptr inbounds i8, i8 addrspace(1)* %arg, i64 undef
+  %tmp3 = load i8, i8 addrspace(1)* %tmp2, align 1
+  %tmp4 = sext i8 %tmp3 to i32
+  %tmp5 = sdiv i32 %tmp1, %tmp4
+  %tmp6 = trunc i32 %tmp5 to i8
+  store i8 %tmp6, i8 addrspace(1)* null, align 1
+  ret void
+}
+