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–1298 ↗

(On Diff #88572)

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

lib/Target/AMDGPU/AMDGPUInstrInfo.td

198 ↗

(On Diff #88572)

Extra newline

lib/Target/AMDGPU/SIISelLowering.cpp

365

Not sure why this appears in the diff

lib/Target/AMDGPU/SIInstructions.td

500–505 ↗

(On Diff #88572)

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

test/CodeGen/AMDGPU/udiv.ll

4–5

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

192–194

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

194

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 ↗

(On Diff #88908)

The casts are unnecessary

lib/Target/AMDGPU/SIInstructions.td

505 ↗

(On Diff #88908)

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

165

Should check the neg source modifier is used

[wdng]

lib/Target/AMDGPU/AMDGPUISelLowering.cpp

1297 ↗

(On Diff #88908)

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 ↗

(On Diff #89399)

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 ↗

(On Diff #89526)

Needs a comment explaining what it is for

lib/Target/AMDGPU/SIInstructions.td

505 ↗

(On Diff #89526)

Extra space after VOP3Mods

[wdng]

Address code review feedback.

[arsenm]

lib/Target/AMDGPU/AMDGPUISelLowering.h

269 ↗

(On Diff #89526)

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 ↗

(On Diff #89682)

should refer to it as ISD::FMAD here

270 ↗

(On Diff #89682)

It's an illegal operation, not an illegal type

[wdng]

Updated, thanks!

[wdng]

ping.

[arsenm]

LGTM

lib/Target/AMDGPU/SIISelLowering.h

class SITargetLowering final : public AMDGPUTargetLowering {
   SDValue LowerLOAD(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerSELECT(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerFastUnsafeFDIV(SDValue Op, SelectionDAG &DAG) const;
   SDValue lowerFDIV_FAST(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFDIV16(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFDIV32(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFDIV64(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFDIV(SDValue Op, SelectionDAG &DAG) const;
+  SDValue LowerFMAD(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerINT_TO_FP(SDValue Op, SelectionDAG &DAG, bool Signed) const;
   SDValue LowerSTORE(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerTrig(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerATOMIC_CMP_SWAP(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerBRCOND(SDValue Op, SelectionDAG &DAG) const;
 
   /// \brief Converts \p Op, which must be of floating point type, to the
   /// floating point type \p VT, by either extending or truncating it.

lib/Target/AMDGPU/SIISelLowering.cpp

SITargetLowering::SITargetLowering(const TargetMachine &TM,
   }
 
   setOperationAction(ISD::FFLOOR, MVT::f64, Legal);
 
   setOperationAction(ISD::FSIN, MVT::f32, Custom);
   setOperationAction(ISD::FCOS, MVT::f32, Custom);
   setOperationAction(ISD::FDIV, MVT::f32, Custom);
   setOperationAction(ISD::FDIV, MVT::f64, Custom);
+  if (Subtarget->hasFP32Denormals())
+    setOperationAction(ISD::FMAD, MVT::f32, Custom);
 
   if (Subtarget->has16BitInsts()) {
     setOperationAction(ISD::Constant, MVT::i16, Legal);
 
     setOperationAction(ISD::SMIN, MVT::i16, Legal);
     setOperationAction(ISD::SMAX, MVT::i16, Legal);
 
     setOperationAction(ISD::UMIN, MVT::i16, Legal);
if (Subtarget->has16BitInsts()) {
     setOperationAction(ISD::SINT_TO_FP, MVT::f16, Promote);
     setOperationAction(ISD::UINT_TO_FP, MVT::f16, Promote);
 
     // F16 - VOP2 Actions.
     setOperationAction(ISD::BR_CC, MVT::f16, Expand);
     setOperationAction(ISD::SELECT_CC, MVT::f16, Expand);
     setOperationAction(ISD::FMAXNUM, MVT::f16, Legal);
     setOperationAction(ISD::FMINNUM, MVT::f16, Legal);
-    setOperationAction(ISD::FDIV, MVT::f16, Custom);
 
     // F16 - VOP3 Actions.
     setOperationAction(ISD::FMA, MVT::f16, Legal);
     if (!Subtarget->hasFP16Denormals())
       setOperationAction(ISD::FMAD, MVT::f16, Legal);
   }
 
   setTargetDAGCombine(ISD::FADD);
case ISD::LOAD: {
     return Result;
   }
 
   case ISD::FSIN:
   case ISD::FCOS:
     return LowerTrig(Op, DAG);
   case ISD::SELECT: return LowerSELECT(Op, DAG);
   case ISD::FDIV: return LowerFDIV(Op, DAG);
+  case ISD::FMAD: return LowerFMAD(Op, DAG);
   case ISD::ATOMIC_CMP_SWAP: return LowerATOMIC_CMP_SWAP(Op, DAG);
   case ISD::STORE: return LowerSTORE(Op, DAG);
   case ISD::GlobalAddress: {
     MachineFunction &MF = DAG.getMachineFunction();
     SIMachineFunctionInfo *MFI = MF.getInfo<SIMachineFunctionInfo>();
     return LowerGlobalAddress(MFI, Op, DAG);
   }
   case ISD::INTRINSIC_WO_CHAIN: return LowerINTRINSIC_WO_CHAIN(Op, DAG);
SDValue SITargetLowering::lowerFDIV_FAST(SDValue Op, SelectionDAG &DAG) const {
   // rcp does not support denormals.
   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);
 }
 
+SDValue SITargetLowering::LowerFMAD(SDValue Op, SelectionDAG &DAG) const {
+  EVT VT = Op.getValueType();
+  SDLoc SL(Op);
+
+  return DAG.getNode(ISD::FMA, SL, VT, Op.getOperand(0), Op.getOperand(1), Op.getOperand(2));
+}
+
+
 SDValue SITargetLowering::LowerFDIV32(SDValue Op, SelectionDAG &DAG) const {
   if (SDValue FastLowered = lowerFastUnsafeFDIV(Op, DAG))
     return FastLowered;
 
   SDLoc SL(Op);
   SDValue LHS = Op.getOperand(0);
   SDValue RHS = Op.getOperand(1);
 

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=r600 -mcpu=redwood < %s | FileCheck -check-prefix=EG -check-prefix=FUNC %s
+; RUN: llc -mtriple=amdgcn--amdhsa -mcpu=fiji -mattr=+fp32-denormals < %s | FileCheck -check-prefix=GCN -check-prefix=DENORM %s
+; RUN: llc -mtriple=amdgcn--amdhsa -mcpu=fiji -mattr=-fp32-denormals < %s | FileCheck -check-prefix=GCN -check-prefix=NODENORM %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

 ; 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_enable_denormals
+; DENORM: v_fma_f32
+define amdgpu_kernel void @fdiv_enable_denormals(i8 addrspace(1)* nocapture readonly %arg) #0 {
+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
+}
+
+; GCN-LABEL: {{^}}fdiv_disable_denormals
+; NODENORM: v_mad_f32
+define amdgpu_kernel void @fdiv_disable_denormals(i8 addrspace(1)* nocapture readonly %arg) #0 {
+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
+}
+
+attributes #0 = { nounwind "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }