[AMDGPU] SelectionDAG divergence tracking should take into account Target divergency. #144947
+21
−15
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-llvm-selectiondag Author: None (alex-t) ChangesIf a kernel is known to be executing only a single lane, IR UniformityAnalysis will take note of that (via GCNTTIImpl::hasBranchDivergence) and report that all values are uniform. SelectionDAG's built-in divergence tracking should do the same. Full diff: https://github.com/llvm/llvm-project/pull/144947.diff 3 Files Affected:
diff --git a/llvm/include/llvm/CodeGen/SelectionDAG.h b/llvm/include/llvm/CodeGen/SelectionDAG.h
index a98e46c587273..66578bdaa2781 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAG.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAG.h
@@ -238,6 +238,8 @@ class SelectionDAG {
LLVMContext *Context;
CodeGenOptLevel OptLevel;
+ bool DivergentTarget = false;
+
UniformityInfo *UA = nullptr;
FunctionLoweringInfo * FLI = nullptr;
@@ -471,14 +473,16 @@ class SelectionDAG {
Pass *PassPtr, const TargetLibraryInfo *LibraryInfo,
UniformityInfo *UA, ProfileSummaryInfo *PSIin,
BlockFrequencyInfo *BFIin, MachineModuleInfo &MMI,
- FunctionVarLocs const *FnVarLocs);
+ FunctionVarLocs const *FnVarLocs, bool HasDivergency);
void init(MachineFunction &NewMF, OptimizationRemarkEmitter &NewORE,
MachineFunctionAnalysisManager &AM,
const TargetLibraryInfo *LibraryInfo, UniformityInfo *UA,
ProfileSummaryInfo *PSIin, BlockFrequencyInfo *BFIin,
- MachineModuleInfo &MMI, FunctionVarLocs const *FnVarLocs) {
- init(NewMF, NewORE, nullptr, LibraryInfo, UA, PSIin, BFIin, MMI, FnVarLocs);
+ MachineModuleInfo &MMI, FunctionVarLocs const *FnVarLocs,
+ bool HasDivergency) {
+ init(NewMF, NewORE, nullptr, LibraryInfo, UA, PSIin, BFIin, MMI, FnVarLocs,
+ HasDivergency);
MFAM = &AM;
}
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 5d8db8be9731f..a63d6561942ad 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -1368,7 +1368,7 @@ void SelectionDAG::init(MachineFunction &NewMF,
const TargetLibraryInfo *LibraryInfo,
UniformityInfo *NewUA, ProfileSummaryInfo *PSIin,
BlockFrequencyInfo *BFIin, MachineModuleInfo &MMIin,
- FunctionVarLocs const *VarLocs) {
+ FunctionVarLocs const *VarLocs, bool HasDivergency) {
MF = &NewMF;
SDAGISelPass = PassPtr;
ORE = &NewORE;
@@ -1381,6 +1381,7 @@ void SelectionDAG::init(MachineFunction &NewMF,
BFI = BFIin;
MMI = &MMIin;
FnVarLocs = VarLocs;
+ DivergentTarget = HasDivergency;
}
SelectionDAG::~SelectionDAG() {
@@ -2327,7 +2328,8 @@ SDValue SelectionDAG::getRegister(Register Reg, EVT VT) {
return SDValue(E, 0);
auto *N = newSDNode<RegisterSDNode>(Reg, VTs);
- N->SDNodeBits.IsDivergent = TLI->isSDNodeSourceOfDivergence(N, FLI, UA);
+ N->SDNodeBits.IsDivergent =
+ DivergentTarget ? TLI->isSDNodeSourceOfDivergence(N, FLI, UA) : false;
CSEMap.InsertNode(N, IP);
InsertNode(N);
return SDValue(N, 0);
@@ -10929,7 +10931,8 @@ SDNode *SelectionDAG::UpdateNodeOperands(SDNode *N, SDValue Op) {
// Now we update the operands.
N->OperandList[0].set(Op);
- updateDivergence(N);
+ if (DivergentTarget)
+ updateDivergence(N);
// If this gets put into a CSE map, add it.
if (InsertPos) CSEMap.InsertNode(N, InsertPos);
return N;
@@ -10958,7 +10961,8 @@ SDNode *SelectionDAG::UpdateNodeOperands(SDNode *N, SDValue Op1, SDValue Op2) {
if (N->OperandList[1] != Op2)
N->OperandList[1].set(Op2);
- updateDivergence(N);
+ if (DivergentTarget)
+ updateDivergence(N);
// If this gets put into a CSE map, add it.
if (InsertPos) CSEMap.InsertNode(N, InsertPos);
return N;
@@ -11009,7 +11013,8 @@ UpdateNodeOperands(SDNode *N, ArrayRef<SDValue> Ops) {
if (N->OperandList[i] != Ops[i])
N->OperandList[i].set(Ops[i]);
- updateDivergence(N);
+ if (DivergentTarget)
+ updateDivergence(N);
// If this gets put into a CSE map, add it.
if (InsertPos) CSEMap.InsertNode(N, InsertPos);
return N;
@@ -11796,8 +11801,9 @@ void SelectionDAG::ReplaceAllUsesWith(SDValue FromN, SDValue To) {
SDUse &Use = *UI;
++UI;
Use.set(To);
- if (To->isDivergent() != From->isDivergent())
- updateDivergence(User);
+ if (DivergentTarget)
+ if (To->isDivergent() != From->isDivergent())
+ updateDivergence(User);
} while (UI != UE && UI->getUser() == User);
// Now that we have modified User, add it back to the CSE maps. If it
// already exists there, recursively merge the results together.
@@ -11854,8 +11860,9 @@ void SelectionDAG::ReplaceAllUsesWith(SDNode *From, SDNode *To) {
SDUse &Use = *UI;
++UI;
Use.setNode(To);
- if (To->isDivergent() != From->isDivergent())
- updateDivergence(User);
+ if (DivergentTarget)
+ if (To->isDivergent() != From->isDivergent())
+ updateDivergence(User);
} while (UI != UE && UI->getUser() == User);
// Now that we have modified User, add it back to the CSE maps. If it
@@ -11907,8 +11914,9 @@ void SelectionDAG::ReplaceAllUsesWith(SDNode *From, const SDValue *To) {
To_IsDivergent |= ToOp->isDivergent();
} while (UI != UE && UI->getUser() == User);
- if (To_IsDivergent != From->isDivergent())
- updateDivergence(User);
+ if (DivergentTarget)
+ if (To_IsDivergent != From->isDivergent())
+ updateDivergence(User);
// Now that we have modified User, add it back to the CSE maps. If it
// already exists there, recursively merge the results together.
@@ -11968,8 +11976,9 @@ void SelectionDAG::ReplaceAllUsesOfValueWith(SDValue From, SDValue To){
++UI;
Use.set(To);
- if (To->isDivergent() != From->isDivergent())
- updateDivergence(User);
+ if (DivergentTarget)
+ if (To->isDivergent() != From->isDivergent())
+ updateDivergence(User);
} while (UI != UE && UI->getUser() == User);
// We are iterating over all uses of the From node, so if a use
// doesn't use the specific value, no changes are made.
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 4b98d87fcc63b..cca314fcda156 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -482,7 +482,10 @@ void SelectionDAGISel::initializeAnalysisResults(
MachineModuleInfo &MMI =
MAMP.getCachedResult<MachineModuleAnalysis>(*Fn.getParent())->getMMI();
- CurDAG->init(*MF, *ORE, MFAM, LibInfo, UA, PSI, BFI, MMI, FnVarLocs);
+ TTI = &FAM.getResult<TargetIRAnalysis>(Fn);
+
+ CurDAG->init(*MF, *ORE, MFAM, LibInfo, UA, PSI, BFI, MMI, FnVarLocs,
+ TTI->hasBranchDivergence());
// Now get the optional analyzes if we want to.
// This is based on the possibly changed OptLevel (after optnone is taken
@@ -500,10 +503,6 @@ void SelectionDAGISel::initializeAnalysisResults(
BatchAA = std::nullopt;
SP = &FAM.getResult<SSPLayoutAnalysis>(Fn);
-
-#if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
- TTI = &FAM.getResult<TargetIRAnalysis>(Fn);
-#endif
}
void SelectionDAGISel::initializeAnalysisResults(MachineFunctionPass &MFP) {
@@ -539,7 +538,10 @@ void SelectionDAGISel::initializeAnalysisResults(MachineFunctionPass &MFP) {
MachineModuleInfo &MMI =
MFP.getAnalysis<MachineModuleInfoWrapperPass>().getMMI();
- CurDAG->init(*MF, *ORE, &MFP, LibInfo, UA, PSI, BFI, MMI, FnVarLocs);
+ TTI = &MFP.getAnalysis<TargetTransformInfoWrapperPass>().getTTI(Fn);
+
+ CurDAG->init(*MF, *ORE, &MFP, LibInfo, UA, PSI, BFI, MMI, FnVarLocs,
+ TTI->hasBranchDivergence());
// Now get the optional analyzes if we want to.
// This is based on the possibly changed OptLevel (after optnone is taken
@@ -558,10 +560,6 @@ void SelectionDAGISel::initializeAnalysisResults(MachineFunctionPass &MFP) {
BatchAA = std::nullopt;
SP = &MFP.getAnalysis<StackProtector>().getLayoutInfo();
-
-#if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
- TTI = &MFP.getAnalysis<TargetTransformInfoWrapperPass>().getTTI(Fn);
-#endif
}
bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
|
jayfoad
reviewed
Jun 20, 2025
I would like to have a test for that but, honestly, I still have no idea how it could look like. For that, I could set the workgroup size to 1 — if only I knew how to check whether the particular code (updateDivergence()) is being executed. |
|
I think you would need a test containing something (probably an intrinsic) that is a source of divergence, but with workgroup size set to 1. Then subsequent operations that depend on the result of that intrinsic will actually be uniform, so the compiler can insert readfirstlane and use SALU instructions for them. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If a kernel is known to be executing only a single lane, IR UniformityAnalysis will take note of that (via GCNTTIImpl::hasBranchDivergence) and report that all values are uniform. SelectionDAG's built-in divergence tracking should do the same.