-
Notifications
You must be signed in to change notification settings - Fork 16.1k
DAG: Remove TypePromoteFloat #177427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DAG: Remove TypePromoteFloat #177427
Conversation
|
@llvm/pr-subscribers-llvm-selectiondag Author: Matt Arsenault (arsenm) ChangesRemove the now unimplemented target hook and associated DAG machinery Really fixes #97975 Full diff: https://github.com/llvm/llvm-project/pull/177427.diff 7 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index d8b5274d37c85..24f8c8952acdf 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -218,15 +218,14 @@ class LLVM_ABI TargetLoweringBase {
TypeScalarizeVector, // Replace this one-element vector with its element.
TypeSplitVector, // Split this vector into two of half the size.
TypeWidenVector, // This vector should be widened into a larger vector.
- TypePromoteFloat, // Replace this float with a larger one.
TypeSoftPromoteHalf, // Soften half to i16 and use float to do arithmetic.
- TypeScalarizeScalableVector, // This action is explicitly left unimplemented.
- // While it is theoretically possible to
- // legalize operations on scalable types with a
- // loop that handles the vscale * #lanes of the
- // vector, this is non-trivial at SelectionDAG
- // level and these types are better to be
- // widened or promoted.
+ TypeScalarizeScalableVector, // This action is explicitly left
+ // unimplemented. While it is theoretically
+ // possible to legalize operations on scalable
+ // types with a loop that handles the vscale *
+ // #lanes of the vector, this is non-trivial at
+ // SelectionDAG level and these types are
+ // better to be widened or promoted.
};
/// LegalizeKind holds the legalization kind that needs to happen to EVT
@@ -546,20 +545,6 @@ class LLVM_ABI TargetLoweringBase {
return TypePromoteInteger;
}
- /// Warning: this option is problem-prone and tends to introduce
- /// float miscompilations:
- ///
- /// - https://github.com/llvm/llvm-project/issues/97975
- /// - https://github.com/llvm/llvm-project/issues/97981
- ///
- /// It should not be overridden to `false` except for special cases.
- ///
- /// Return true if the half type should be promoted using soft promotion rules
- /// where each operation is promoted to f32 individually, then converted to
- /// fp16. The default behavior is to promote chains of operations, keeping
- /// intermediate results in f32 precision and range.
- virtual bool softPromoteHalfType() const { return true; }
-
// Return true if, for soft-promoted half, the half type should be passed to
// and returned from functions as f32. The default behavior is to pass as
// i16. If soft-promoted half is not used, this function is ignored and
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
index 1e7bc757d2c58..ce952d50c287c 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
@@ -634,17 +634,6 @@ SDValue DAGTypeLegalizer::SoftenFloatRes_FP_EXTEND(SDNode *N) {
SDValue Chain = IsStrict ? N->getOperand(0) : SDValue();
- if (getTypeAction(Op.getValueType()) == TargetLowering::TypePromoteFloat) {
- Op = GetPromotedFloat(Op);
- // If the promotion did the FP_EXTEND to the destination type for us,
- // there's nothing left to do here.
- if (Op.getValueType() == N->getValueType(0)) {
- if (IsStrict)
- ReplaceValueWith(SDValue(N, 1), Chain);
- return BitConvertToInteger(Op);
- }
- }
-
// There's only a libcall for f16 -> f32 and shifting is only valid for bf16
// -> f32, so proceed in two stages. Also, it's entirely possible for both
// f16 and f32 to be legal, so use the fully hard-float FP_EXTEND rather
@@ -3298,8 +3287,6 @@ SDValue DAGTypeLegalizer::PromoteFloatRes_VECREDUCE_SEQ(SDNode *N) {
}
SDValue DAGTypeLegalizer::BitcastToInt_ATOMIC_SWAP(SDNode *N) {
- EVT VT = N->getValueType(0);
-
AtomicSDNode *AM = cast<AtomicSDNode>(N);
SDLoc SL(N);
@@ -3314,18 +3301,11 @@ SDValue DAGTypeLegalizer::BitcastToInt_ATOMIC_SWAP(SDNode *N) {
SDValue Result = NewAtomic;
- if (getTypeAction(VT) == TargetLowering::TypePromoteFloat) {
- EVT NFPVT = TLI.getTypeToTransformTo(*DAG.getContext(), VT);
- Result = DAG.getNode(GetPromotionOpcode(VT, NFPVT), SL, NFPVT,
- NewAtomic);
- }
-
// Legalize the chain result by replacing uses of the old value chain with the
// new one
ReplaceValueWith(SDValue(N, 1), NewAtomic.getValue(1));
return Result;
-
}
//===----------------------------------------------------------------------===//
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
index faa6a1d0d14a3..8ce41df6be69b 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
@@ -534,12 +534,6 @@ SDValue DAGTypeLegalizer::PromoteIntRes_BITCAST(SDNode *N) {
case TargetLowering::TypeSoftPromoteHalf:
// Promote the integer operand by hand.
return DAG.getNode(ISD::ANY_EXTEND, dl, NOutVT, GetSoftPromotedHalf(InOp));
- case TargetLowering::TypePromoteFloat: {
- // Convert the promoted float by hand.
- if (!NOutVT.isVector())
- return DAG.getNode(ISD::FP_TO_FP16, dl, NOutVT, GetPromotedFloat(InOp));
- break;
- }
case TargetLowering::TypeExpandInteger:
case TargetLowering::TypeExpandFloat:
break;
@@ -4250,8 +4244,6 @@ void DAGTypeLegalizer::ExpandIntRes_FP_TO_XINT(SDNode *N, SDValue &Lo,
bool IsStrict = N->isStrictFPOpcode();
SDValue Chain = IsStrict ? N->getOperand(0) : SDValue();
SDValue Op = N->getOperand(IsStrict ? 1 : 0);
- if (getTypeAction(Op.getValueType()) == TargetLowering::TypePromoteFloat)
- Op = GetPromotedFloat(Op);
// If the input is bf16 or needs to be soft promoted, extend to f32.
if (getTypeAction(Op.getValueType()) == TargetLowering::TypeSoftPromoteHalf ||
@@ -4292,9 +4284,6 @@ void DAGTypeLegalizer::ExpandIntRes_XROUND_XRINT(SDNode *N, SDValue &Lo,
SDValue Op = N->getOperand(IsStrict ? 1 : 0);
SDValue Chain = IsStrict ? N->getOperand(0) : SDValue();
- assert(getTypeAction(Op.getValueType()) != TargetLowering::TypePromoteFloat &&
- "Input type needs to be promoted!");
-
EVT VT = Op.getValueType();
if (VT == MVT::f16) {
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
index f14eeda639e71..9634e5afdc738 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
@@ -288,10 +288,6 @@ bool DAGTypeLegalizer::run() {
WidenVectorResult(N, i);
Changed = true;
goto NodeDone;
- case TargetLowering::TypePromoteFloat:
- PromoteFloatResult(N, i);
- Changed = true;
- goto NodeDone;
case TargetLowering::TypeSoftPromoteHalf:
SoftPromoteHalfResult(N, i);
Changed = true;
@@ -351,10 +347,6 @@ bool DAGTypeLegalizer::run() {
NeedsReanalyzing = WidenVectorOperand(N, i);
Changed = true;
break;
- case TargetLowering::TypePromoteFloat:
- NeedsReanalyzing = PromoteFloatOperand(N, i);
- Changed = true;
- break;
case TargetLowering::TypeSoftPromoteHalf:
NeedsReanalyzing = SoftPromoteHalfOperand(N, i);
Changed = true;
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp
index 88c1af20a321e..6ded0bf0a92c0 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypesGeneric.cpp
@@ -49,7 +49,6 @@ void DAGTypeLegalizer::ExpandRes_BITCAST(SDNode *N, SDValue &Lo, SDValue &Hi) {
case TargetLowering::TypeLegal:
case TargetLowering::TypePromoteInteger:
break;
- case TargetLowering::TypePromoteFloat:
case TargetLowering::TypeSoftPromoteHalf:
llvm_unreachable("Bitcast of a promotion-needing float should never need"
"expansion");
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
index 1a7cec8fc7565..ac8b04a1aecb2 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
@@ -1660,7 +1660,6 @@ void DAGTypeLegalizer::SplitVecRes_BITCAST(SDNode *N, SDValue &Lo,
switch (getTypeAction(InVT)) {
case TargetLowering::TypeLegal:
case TargetLowering::TypePromoteInteger:
- case TargetLowering::TypePromoteFloat:
case TargetLowering::TypeSoftPromoteHalf:
case TargetLowering::TypeSoftenFloat:
case TargetLowering::TypeScalarizeVector:
@@ -6022,7 +6021,6 @@ SDValue DAGTypeLegalizer::WidenVecRes_BITCAST(SDNode *N) {
break;
}
case TargetLowering::TypeSoftenFloat:
- case TargetLowering::TypePromoteFloat:
case TargetLowering::TypeSoftPromoteHalf:
case TargetLowering::TypeExpandInteger:
case TargetLowering::TypeExpandFloat:
diff --git a/llvm/lib/CodeGen/TargetLoweringBase.cpp b/llvm/lib/CodeGen/TargetLoweringBase.cpp
index 922b3fa40518d..c6707162d9b9e 100644
--- a/llvm/lib/CodeGen/TargetLoweringBase.cpp
+++ b/llvm/lib/CodeGen/TargetLoweringBase.cpp
@@ -1749,8 +1749,7 @@ void TargetLoweringBase::computeRegisterProperties(
// conversions).
if (!isTypeLegal(MVT::f16)) {
// Allow targets to control how we legalize half.
- bool SoftPromoteHalfType = softPromoteHalfType();
- bool UseFPRegsForHalfType = !SoftPromoteHalfType || useFPRegsForHalfType();
+ bool UseFPRegsForHalfType = useFPRegsForHalfType();
if (!UseFPRegsForHalfType) {
NumRegistersForVT[MVT::f16] = NumRegistersForVT[MVT::i16];
@@ -1760,11 +1759,7 @@ void TargetLoweringBase::computeRegisterProperties(
RegisterTypeForVT[MVT::f16] = RegisterTypeForVT[MVT::f32];
}
TransformToType[MVT::f16] = MVT::f32;
- if (SoftPromoteHalfType) {
- ValueTypeActions.setTypeAction(MVT::f16, TypeSoftPromoteHalf);
- } else {
- ValueTypeActions.setTypeAction(MVT::f16, TypePromoteFloat);
- }
+ ValueTypeActions.setTypeAction(MVT::f16, TypeSoftPromoteHalf);
}
// Decide how to handle bf16. If the target does not have native bf16 support,
|
|
Maybe a better title is "Remove TypePromoteFloat". So it's in terms of the promotion action being removed rather than the name of the target hook. Is misinterpreted the title at first. |
🪟 Windows x64 Test Results
All executed tests passed, but another part of the build failed. Click on a failure below to see the details. [code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600MachineCFGStructurizer.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/AMDGPUSubtarget.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600ClauseMergePass.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600AsmPrinter.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600ControlFlowFinalizer.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600EmitClauseMarkers.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600ExpandSpecialInstrs.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600FrameLowering.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600InstrInfo.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600MachineFunctionInfo.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600MachineScheduler.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600OptimizeVectorRegisters.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600RegisterInfo.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600Packetizer.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600Subtarget.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600MCInstLower.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600ISelLowering.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/AMDGPUTargetMachine.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600TargetTransformInfo.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600ISelDAGToDAG.cpp.obj[code=1] lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600TargetMachine.cpp.objIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
5d1b80a to
8e53101
Compare
6025178 to
074f0b4
Compare
074f0b4 to
febe138
Compare
8e53101 to
854d088
Compare
shiltian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to deprecate it first in case of any downstream users?
| // types with a loop that handles the vscale * | ||
| // #lanes of the vector, this is non-trivial at | ||
| // SelectionDAG level and these types are | ||
| // better to be widened or promoted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't seem like there is anything changed here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang-format wanted to reindent everything
febe138 to
c91ea57
Compare
854d088 to
10ef135
Compare
No |
RKSimon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - but please add a release note mentioning that its now removed
10ef135 to
5582f29
Compare
🐧 Linux x64 Test Results
All executed tests passed, but another part of the build failed. Click on a failure below to see the details. lib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600MachineCFGStructurizer.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600ClauseMergePass.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600EmitClauseMarkers.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600ControlFlowFinalizer.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600ExpandSpecialInstrs.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600FrameLowering.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/AMDGPUSubtarget.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600AsmPrinter.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600InstrInfo.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600MachineFunctionInfo.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600OptimizeVectorRegisters.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600RegisterInfo.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600MachineScheduler.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600Packetizer.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600Subtarget.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600MCInstLower.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600ISelLowering.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600TargetTransformInfo.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600ISelDAGToDAG.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/R600TargetMachine.cpp.olib/Target/AMDGPU/CMakeFiles/LLVMAMDGPUCodeGen.dir/AMDGPUTargetMachine.cpp.oIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
Remove the now unimplemented target hook and associated DAG machinery for the old half legalization path. Really fixes #97975
5582f29 to
be21516
Compare
Remove the now unimplemented target hook and associated DAG machinery for the old half legalization path. Really fixes llvm#97975
Remove the now unimplemented target hook and associated DAG machinery for the old half legalization path. Really fixes llvm#97975

Remove the now unimplemented target hook and associated DAG machinery
for the old half legalization path.
Really fixes #97975