[NFC][AMDGPU] Let IR level callers query the FMA/FMAD predicates - #213310
Conversation
isFMADLegal and isFMAFasterThanFMulAndFAdd read the denormal mode out of the MachineFunction, so nothing before instruction selection can ask them whether a given fmul/fadd pair will be fused. Split the denormal mode out into explicit arguments and make the existing entry points thin wrappers. A small refactoring prior changes in getArithmeticInstrCost and isProfitableToSinkOperands. Contributes to llvm#211092 Assisted-By: Claude Opus 5
|
@llvm/pr-subscribers-backend-amdgpu Author: Dmitry Sidorov (MrSidims) ChangesisFMADLegal and isFMAFasterThanFMulAndFAdd read the denormal mode out of the MachineFunction, so nothing before instruction selection can ask them whether a given fmul/fadd pair will be fused. Split the denormal mode out into explicit arguments and make the existing entry points thin wrappers. A small refactoring prior changes in getArithmeticInstrCost and isProfitableToSinkOperands. Contributes to #211092 Assisted-By: Claude Opus 5 Full diff: https://github.com/llvm/llvm-project/pull/213310.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
index b5e2a36ad9f19..8614ffbd2d1fa 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.cpp
@@ -7420,8 +7420,8 @@ LLT SITargetLowering::getPreferredShiftAmountTy(LLT Ty) const {
// however does not support denormals, so we do report fma as faster if we have
// a fast fma device and require denormals.
//
-bool SITargetLowering::isFMAFasterThanFMulAndFAdd(const MachineFunction &MF,
- EVT VT) const {
+bool SITargetLowering::isFMAFasterThanFMulAndFAdd(
+ EVT VT, bool FlushF32Denormals, bool FlushF64F16Denormals) const {
VT = VT.getScalarType();
switch (VT.getSimpleVT().SimpleTy) {
@@ -7433,7 +7433,7 @@ bool SITargetLowering::isFMAFasterThanFMulAndFAdd(const MachineFunction &MF,
// Otherwise f32 mad is always full rate and returns the same result as
// the separate operations so should be preferred over fma.
// However does not support denormals.
- if (!denormalModeIsFlushAllF32(MF))
+ if (!FlushF32Denormals)
return Subtarget->hasFastFMAF32() || Subtarget->hasDLInsts();
// If the subtarget has v_fmac_f32, that's just as good as v_mac_f32.
@@ -7443,7 +7443,7 @@ bool SITargetLowering::isFMAFasterThanFMulAndFAdd(const MachineFunction &MF,
return true;
case MVT::f16:
case MVT::bf16:
- return Subtarget->has16BitInsts() && !denormalModeIsFlushAllF64F16(MF);
+ return Subtarget->has16BitInsts() && !FlushF64F16Denormals;
default:
break;
}
@@ -7451,6 +7451,12 @@ bool SITargetLowering::isFMAFasterThanFMulAndFAdd(const MachineFunction &MF,
return false;
}
+bool SITargetLowering::isFMAFasterThanFMulAndFAdd(const MachineFunction &MF,
+ EVT VT) const {
+ return isFMAFasterThanFMulAndFAdd(VT, denormalModeIsFlushAllF32(MF),
+ denormalModeIsFlushAllF64F16(MF));
+}
+
bool SITargetLowering::isFMAFasterThanFMulAndFAdd(const MachineFunction &MF,
LLT Ty) const {
switch (Ty.getScalarSizeInBits()) {
@@ -7467,33 +7473,38 @@ bool SITargetLowering::isFMAFasterThanFMulAndFAdd(const MachineFunction &MF,
return false;
}
+bool SITargetLowering::isFMADLegal(EVT VT, bool FlushF32Denormals,
+ bool FlushF64F16Denormals) const {
+ // TODO: Check future ftz flag
+ // v_mad_f32/v_mac_f32 do not support denormals.
+ if (VT == MVT::f32)
+ return Subtarget->hasMadMacF32Insts() && FlushF32Denormals;
+ if (VT == MVT::f16)
+ return Subtarget->hasMadF16() && FlushF64F16Denormals;
+
+ return false;
+}
+
bool SITargetLowering::isFMADLegal(const MachineInstr &MI, LLT Ty) const {
if (!Ty.isScalar())
return false;
+ const MachineFunction &MF = *MI.getMF();
if (Ty.getScalarSizeInBits() == 16)
- return Subtarget->hasMadF16() && denormalModeIsFlushAllF64F16(*MI.getMF());
+ return isFMADLegal(MVT::f16, denormalModeIsFlushAllF32(MF),
+ denormalModeIsFlushAllF64F16(MF));
if (Ty.getScalarSizeInBits() == 32)
- return Subtarget->hasMadMacF32Insts() &&
- denormalModeIsFlushAllF32(*MI.getMF());
+ return isFMADLegal(MVT::f32, denormalModeIsFlushAllF32(MF),
+ denormalModeIsFlushAllF64F16(MF));
return false;
}
bool SITargetLowering::isFMADLegal(const SelectionDAG &DAG,
const SDNode *N) const {
- // TODO: Check future ftz flag
- // v_mad_f32/v_mac_f32 do not support denormals.
- EVT VT = N->getValueType(0);
- if (VT == MVT::f32)
- return Subtarget->hasMadMacF32Insts() &&
- denormalModeIsFlushAllF32(DAG.getMachineFunction());
- if (VT == MVT::f16) {
- return Subtarget->hasMadF16() &&
- denormalModeIsFlushAllF64F16(DAG.getMachineFunction());
- }
-
- return false;
+ const MachineFunction &MF = DAG.getMachineFunction();
+ return isFMADLegal(N->getValueType(0), denormalModeIsFlushAllF32(MF),
+ denormalModeIsFlushAllF64F16(MF));
}
//===----------------------------------------------------------------------===//
diff --git a/llvm/lib/Target/AMDGPU/SIISelLowering.h b/llvm/lib/Target/AMDGPU/SIISelLowering.h
index be4bb6d825b46..c8f4c65e55193 100644
--- a/llvm/lib/Target/AMDGPU/SIISelLowering.h
+++ b/llvm/lib/Target/AMDGPU/SIISelLowering.h
@@ -503,6 +503,14 @@ class SITargetLowering final : public AMDGPUTargetLowering {
bool isFMADLegal(const SelectionDAG &DAG, const SDNode *N) const override;
bool isFMADLegal(const MachineInstr &MI, const LLT Ty) const override;
+ /// Variants taking the denormal mode directly, for IR level callers which
+ /// have no MachineFunction to read it from. \p VT is the legalized type of
+ /// the operation.
+ bool isFMAFasterThanFMulAndFAdd(EVT VT, bool FlushF32Denormals,
+ bool FlushF64F16Denormals) const;
+ bool isFMADLegal(EVT VT, bool FlushF32Denormals,
+ bool FlushF64F16Denormals) const;
+
SDValue splitUnaryVectorOp(SDValue Op, SelectionDAG &DAG) const;
SDValue splitBinaryVectorOp(SDValue Op, SelectionDAG &DAG) const;
SDValue splitTernaryVectorOp(SDValue Op, SelectionDAG &DAG) const;
|
| bool isFMADLegal(const MachineInstr &MI, const LLT Ty) const override; | ||
|
|
||
| /// Variants taking the denormal mode directly, for IR level callers which | ||
| /// have no MachineFunction to read it from. \p VT is the legalized type of |
There was a problem hiding this comment.
The IR version has a Function which has the FP mode?
There was a problem hiding this comment.
Indeed, I'll replace the bool variants with isFMAFasterThanFMulAndFAdd(const Function &F, EVT VT) / isFMADLegal(const Function &F, EVT VT), which build the SIModeRegisterDefaults from F themselves.
| // | ||
| bool SITargetLowering::isFMAFasterThanFMulAndFAdd( | ||
| EVT VT, bool FlushF32Denormals, bool FlushF64F16Denormals) const { | ||
| EVT VT, const SIModeRegisterDefaults &Mode) const { |
There was a problem hiding this comment.
I'd rather keep this in terms of DenormalFPEnv or the machine function, not spread references to SIModeRegisterDefaults
There was a problem hiding this comment.
Done, both predicates now take a DenormalFPEnv
isFMADLegal and isFMAFasterThanFMulAndFAdd read the denormal mode out of the MachineFunction, so nothing before instruction selection can ask them whether an fmul/fadd pair will be fused. Take an explicit DenormalFPEnv instead, and make the existing MachineFunction / SelectionDAG / MachineInstr entry points thin wrappers over it.
Also override the IR level isFMAFasterThanFMulAndFAdd hook. The two views agree by construction, since SIModeRegisterDefaults copies its denormal fields out of getDenormalFPEnv.
isFMADLegal uses VT as written and does not look through vectors, so a vector type reports false, as in the SelectionDAG overload it was extracted from.
The patch is preparation for querying these from getArithmeticInstrCost and a revived isProfitableToSinkOperands.
Contributes to #211092
Assisted-By: Claude Opus 5