[Uniformity] Remove dependence on CycleInfo in old pass manager #76011
Conversation
This file contains 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
UniformityAnalysis depends on CycleInfo, but when a transform "preserves" on UniformityAnalysis, the transitive dependence can causes issues in the pass manager. The correct way to fix this would be to also update and preserve CycleInfo. But instead of that, we prefer to just recompute CycleInfo for UniformityAnalysis because the situation is rare enough.
|
@llvm/pr-subscribers-llvm-analysis @llvm/pr-subscribers-backend-amdgpu Author: Sameer Sahasrabuddhe (ssahasra) ChangesUniformityAnalysis depends on CycleInfo, but when a transform "preserves" on UniformityAnalysis, the transitive dependence can causes issues in the pass manager. The correct way to fix this would be to also update and preserve CycleInfo. But instead of that, we prefer to just recompute CycleInfo for UniformityAnalysis because the situation is rare enough. Full diff: https://github.com/llvm/llvm-project/pull/76011.diff 3 Files Affected:
diff --git a/llvm/include/llvm/Analysis/UniformityAnalysis.h b/llvm/include/llvm/Analysis/UniformityAnalysis.h
index f42c4950ed649f..44f7840dc8e144 100644
--- a/llvm/include/llvm/Analysis/UniformityAnalysis.h
+++ b/llvm/include/llvm/Analysis/UniformityAnalysis.h
@@ -52,6 +52,7 @@ class UniformityInfoPrinterPass
/// Legacy analysis pass which computes a \ref CycleInfo.
class UniformityInfoWrapperPass : public FunctionPass {
Function *m_function = nullptr;
+ CycleInfo m_cycleInfo;
UniformityInfo m_uniformityInfo;
public:
diff --git a/llvm/lib/Analysis/UniformityAnalysis.cpp b/llvm/lib/Analysis/UniformityAnalysis.cpp
index 2d617db431c588..3e40b7d8d9fc02 100644
--- a/llvm/lib/Analysis/UniformityAnalysis.cpp
+++ b/llvm/lib/Analysis/UniformityAnalysis.cpp
@@ -152,7 +152,6 @@ UniformityInfoWrapperPass::UniformityInfoWrapperPass() : FunctionPass(ID) {
INITIALIZE_PASS_BEGIN(UniformityInfoWrapperPass, "uniformity",
"Uniformity Analysis", true, true)
INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
-INITIALIZE_PASS_DEPENDENCY(CycleInfoWrapperPass)
INITIALIZE_PASS_DEPENDENCY(TargetTransformInfoWrapperPass)
INITIALIZE_PASS_END(UniformityInfoWrapperPass, "uniformity",
"Uniformity Analysis", true, true)
@@ -160,18 +159,27 @@ INITIALIZE_PASS_END(UniformityInfoWrapperPass, "uniformity",
void UniformityInfoWrapperPass::getAnalysisUsage(AnalysisUsage &AU) const {
AU.setPreservesAll();
AU.addRequired<DominatorTreeWrapperPass>();
- AU.addRequiredTransitive<CycleInfoWrapperPass>();
AU.addRequired<TargetTransformInfoWrapperPass>();
}
bool UniformityInfoWrapperPass::runOnFunction(Function &F) {
- auto &cycleInfo = getAnalysis<CycleInfoWrapperPass>().getResult();
auto &domTree = getAnalysis<DominatorTreeWrapperPass>().getDomTree();
auto &targetTransformInfo =
getAnalysis<TargetTransformInfoWrapperPass>().getTTI(F);
m_function = &F;
- m_uniformityInfo = UniformityInfo{domTree, cycleInfo, &targetTransformInfo};
+
+ // FIXME: CycleInfo is a transitive dependency for UniformityInfo, and we
+ // would have liked to say that CycleInfo is also preserved whenever
+ // UniformityInfo is preserved. But some passes like
+ // AMDGPUUnifyDivergentExitNodes do not update cycle membership and lists of
+ // cycle exits, since that facility is not available. The end result is that
+ // we cannot declare CycleInfo as a transitive depedency for UniformityInfo.
+ // Instead we recompute CycleInfo on demand. This is OK because it is rarely
+ // needed, and only affects the old pass manager.
+ m_cycleInfo.clear();
+ m_cycleInfo.compute(F);
+ m_uniformityInfo = UniformityInfo{domTree, m_cycleInfo, &targetTransformInfo};
// Skip computation if we can assume everything is uniform.
if (targetTransformInfo.hasBranchDivergence(m_function))
diff --git a/llvm/test/CodeGen/AMDGPU/llc-pipeline.ll b/llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
index 8b0b6263832243..e0fcc31cc215a7 100644
--- a/llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
+++ b/llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
@@ -57,7 +57,6 @@
; GCN-O0-NEXT: Remove unreachable blocks from the CFG
; GCN-O0-NEXT: Post-Dominator Tree Construction
; GCN-O0-NEXT: Dominator Tree Construction
-; GCN-O0-NEXT: Cycle Info Analysis
; GCN-O0-NEXT: Uniformity Analysis
; GCN-O0-NEXT: Unify divergent function exit nodes
; GCN-O0-NEXT: Dominator Tree Construction
@@ -69,7 +68,6 @@
; GCN-O0-NEXT: Detect single entry single exit regions
; GCN-O0-NEXT: Region Pass Manager
; GCN-O0-NEXT: Structurize control flow
-; GCN-O0-NEXT: Cycle Info Analysis
; GCN-O0-NEXT: Uniformity Analysis
; GCN-O0-NEXT: Basic Alias Analysis (stateless AA impl)
; GCN-O0-NEXT: Function Alias Analysis Results
@@ -77,7 +75,6 @@
; GCN-O0-NEXT: AMDGPU Annotate Uniform Values
; GCN-O0-NEXT: Natural Loop Information
; GCN-O0-NEXT: SI annotate control flow
-; GCN-O0-NEXT: Cycle Info Analysis
; GCN-O0-NEXT: Uniformity Analysis
; GCN-O0-NEXT: AMDGPU Rewrite Undef for PHI
; GCN-O0-NEXT: LCSSA Verifier
@@ -88,7 +85,6 @@
; GCN-O0-NEXT: Safe Stack instrumentation pass
; GCN-O0-NEXT: Insert stack protectors
; GCN-O0-NEXT: Dominator Tree Construction
-; GCN-O0-NEXT: Cycle Info Analysis
; GCN-O0-NEXT: Uniformity Analysis
; GCN-O0-NEXT: Assignment Tracking Analysis
; GCN-O0-NEXT: AMDGPU DAG->DAG Pattern Instruction Selection
@@ -186,13 +182,11 @@
; GCN-O1-NEXT: FunctionPass Manager
; GCN-O1-NEXT: Infer address spaces
; GCN-O1-NEXT: Dominator Tree Construction
-; GCN-O1-NEXT: Cycle Info Analysis
; GCN-O1-NEXT: Uniformity Analysis
; GCN-O1-NEXT: AMDGPU atomic optimizations
; GCN-O1-NEXT: Expand Atomic instructions
; GCN-O1-NEXT: AMDGPU Promote Alloca
; GCN-O1-NEXT: Dominator Tree Construction
-; GCN-O1-NEXT: Cycle Info Analysis
; GCN-O1-NEXT: Uniformity Analysis
; GCN-O1-NEXT: AMDGPU IR optimizations
; GCN-O1-NEXT: Basic Alias Analysis (stateless AA impl)
@@ -241,7 +235,6 @@
; GCN-O1-NEXT: Function Alias Analysis Results
; GCN-O1-NEXT: Flatten the CFG
; GCN-O1-NEXT: Dominator Tree Construction
-; GCN-O1-NEXT: Cycle Info Analysis
; GCN-O1-NEXT: Uniformity Analysis
; GCN-O1-NEXT: AMDGPU IR late optimizations
; GCN-O1-NEXT: Basic Alias Analysis (stateless AA impl)
@@ -259,7 +252,6 @@
; GCN-O1-NEXT: Detect single entry single exit regions
; GCN-O1-NEXT: Region Pass Manager
; GCN-O1-NEXT: Structurize control flow
-; GCN-O1-NEXT: Cycle Info Analysis
; GCN-O1-NEXT: Uniformity Analysis
; GCN-O1-NEXT: Basic Alias Analysis (stateless AA impl)
; GCN-O1-NEXT: Function Alias Analysis Results
@@ -267,7 +259,6 @@
; GCN-O1-NEXT: AMDGPU Annotate Uniform Values
; GCN-O1-NEXT: Natural Loop Information
; GCN-O1-NEXT: SI annotate control flow
-; GCN-O1-NEXT: Cycle Info Analysis
; GCN-O1-NEXT: Uniformity Analysis
; GCN-O1-NEXT: AMDGPU Rewrite Undef for PHI
; GCN-O1-NEXT: LCSSA Verifier
@@ -278,7 +269,6 @@
; GCN-O1-NEXT: Safe Stack instrumentation pass
; GCN-O1-NEXT: Insert stack protectors
; GCN-O1-NEXT: Dominator Tree Construction
-; GCN-O1-NEXT: Cycle Info Analysis
; GCN-O1-NEXT: Uniformity Analysis
; GCN-O1-NEXT: Basic Alias Analysis (stateless AA impl)
; GCN-O1-NEXT: Function Alias Analysis Results
@@ -454,7 +444,6 @@
; GCN-O1-OPTS-NEXT: FunctionPass Manager
; GCN-O1-OPTS-NEXT: Infer address spaces
; GCN-O1-OPTS-NEXT: Dominator Tree Construction
-; GCN-O1-OPTS-NEXT: Cycle Info Analysis
; GCN-O1-OPTS-NEXT: Uniformity Analysis
; GCN-O1-OPTS-NEXT: AMDGPU atomic optimizations
; GCN-O1-OPTS-NEXT: Expand Atomic instructions
@@ -474,7 +463,6 @@
; GCN-O1-OPTS-NEXT: Scalar Evolution Analysis
; GCN-O1-OPTS-NEXT: Nary reassociation
; GCN-O1-OPTS-NEXT: Early CSE
-; GCN-O1-OPTS-NEXT: Cycle Info Analysis
; GCN-O1-OPTS-NEXT: Uniformity Analysis
; GCN-O1-OPTS-NEXT: AMDGPU IR optimizations
; GCN-O1-OPTS-NEXT: Basic Alias Analysis (stateless AA impl)
@@ -529,7 +517,6 @@
; GCN-O1-OPTS-NEXT: Function Alias Analysis Results
; GCN-O1-OPTS-NEXT: Flatten the CFG
; GCN-O1-OPTS-NEXT: Dominator Tree Construction
-; GCN-O1-OPTS-NEXT: Cycle Info Analysis
; GCN-O1-OPTS-NEXT: Uniformity Analysis
; GCN-O1-OPTS-NEXT: AMDGPU IR late optimizations
; GCN-O1-OPTS-NEXT: Basic Alias Analysis (stateless AA impl)
@@ -547,7 +534,6 @@
; GCN-O1-OPTS-NEXT: Detect single entry single exit regions
; GCN-O1-OPTS-NEXT: Region Pass Manager
; GCN-O1-OPTS-NEXT: Structurize control flow
-; GCN-O1-OPTS-NEXT: Cycle Info Analysis
; GCN-O1-OPTS-NEXT: Uniformity Analysis
; GCN-O1-OPTS-NEXT: Basic Alias Analysis (stateless AA impl)
; GCN-O1-OPTS-NEXT: Function Alias Analysis Results
@@ -555,7 +541,6 @@
; GCN-O1-OPTS-NEXT: AMDGPU Annotate Uniform Values
; GCN-O1-OPTS-NEXT: Natural Loop Information
; GCN-O1-OPTS-NEXT: SI annotate control flow
-; GCN-O1-OPTS-NEXT: Cycle Info Analysis
; GCN-O1-OPTS-NEXT: Uniformity Analysis
; GCN-O1-OPTS-NEXT: AMDGPU Rewrite Undef for PHI
; GCN-O1-OPTS-NEXT: LCSSA Verifier
@@ -566,7 +551,6 @@
; GCN-O1-OPTS-NEXT: Safe Stack instrumentation pass
; GCN-O1-OPTS-NEXT: Insert stack protectors
; GCN-O1-OPTS-NEXT: Dominator Tree Construction
-; GCN-O1-OPTS-NEXT: Cycle Info Analysis
; GCN-O1-OPTS-NEXT: Uniformity Analysis
; GCN-O1-OPTS-NEXT: Basic Alias Analysis (stateless AA impl)
; GCN-O1-OPTS-NEXT: Function Alias Analysis Results
@@ -752,7 +736,6 @@
; GCN-O2-NEXT: FunctionPass Manager
; GCN-O2-NEXT: Infer address spaces
; GCN-O2-NEXT: Dominator Tree Construction
-; GCN-O2-NEXT: Cycle Info Analysis
; GCN-O2-NEXT: Uniformity Analysis
; GCN-O2-NEXT: AMDGPU atomic optimizations
; GCN-O2-NEXT: Expand Atomic instructions
@@ -766,7 +749,6 @@
; GCN-O2-NEXT: Scalar Evolution Analysis
; GCN-O2-NEXT: Nary reassociation
; GCN-O2-NEXT: Early CSE
-; GCN-O2-NEXT: Cycle Info Analysis
; GCN-O2-NEXT: Uniformity Analysis
; GCN-O2-NEXT: AMDGPU IR optimizations
; GCN-O2-NEXT: Basic Alias Analysis (stateless AA impl)
@@ -829,7 +811,6 @@
; GCN-O2-NEXT: Function Alias Analysis Results
; GCN-O2-NEXT: Flatten the CFG
; GCN-O2-NEXT: Dominator Tree Construction
-; GCN-O2-NEXT: Cycle Info Analysis
; GCN-O2-NEXT: Uniformity Analysis
; GCN-O2-NEXT: AMDGPU IR late optimizations
; GCN-O2-NEXT: Basic Alias Analysis (stateless AA impl)
@@ -847,7 +828,6 @@
; GCN-O2-NEXT: Detect single entry single exit regions
; GCN-O2-NEXT: Region Pass Manager
; GCN-O2-NEXT: Structurize control flow
-; GCN-O2-NEXT: Cycle Info Analysis
; GCN-O2-NEXT: Uniformity Analysis
; GCN-O2-NEXT: Basic Alias Analysis (stateless AA impl)
; GCN-O2-NEXT: Function Alias Analysis Results
@@ -855,7 +835,6 @@
; GCN-O2-NEXT: AMDGPU Annotate Uniform Values
; GCN-O2-NEXT: Natural Loop Information
; GCN-O2-NEXT: SI annotate control flow
-; GCN-O2-NEXT: Cycle Info Analysis
; GCN-O2-NEXT: Uniformity Analysis
; GCN-O2-NEXT: AMDGPU Rewrite Undef for PHI
; GCN-O2-NEXT: LCSSA Verifier
@@ -867,7 +846,6 @@
; GCN-O2-NEXT: Safe Stack instrumentation pass
; GCN-O2-NEXT: Insert stack protectors
; GCN-O2-NEXT: Dominator Tree Construction
-; GCN-O2-NEXT: Cycle Info Analysis
; GCN-O2-NEXT: Uniformity Analysis
; GCN-O2-NEXT: Basic Alias Analysis (stateless AA impl)
; GCN-O2-NEXT: Function Alias Analysis Results
@@ -1054,7 +1032,6 @@
; GCN-O3-NEXT: FunctionPass Manager
; GCN-O3-NEXT: Infer address spaces
; GCN-O3-NEXT: Dominator Tree Construction
-; GCN-O3-NEXT: Cycle Info Analysis
; GCN-O3-NEXT: Uniformity Analysis
; GCN-O3-NEXT: AMDGPU atomic optimizations
; GCN-O3-NEXT: Expand Atomic instructions
@@ -1074,7 +1051,6 @@
; GCN-O3-NEXT: Scalar Evolution Analysis
; GCN-O3-NEXT: Nary reassociation
; GCN-O3-NEXT: Early CSE
-; GCN-O3-NEXT: Cycle Info Analysis
; GCN-O3-NEXT: Uniformity Analysis
; GCN-O3-NEXT: AMDGPU IR optimizations
; GCN-O3-NEXT: Basic Alias Analysis (stateless AA impl)
@@ -1143,7 +1119,6 @@
; GCN-O3-NEXT: Function Alias Analysis Results
; GCN-O3-NEXT: Flatten the CFG
; GCN-O3-NEXT: Dominator Tree Construction
-; GCN-O3-NEXT: Cycle Info Analysis
; GCN-O3-NEXT: Uniformity Analysis
; GCN-O3-NEXT: AMDGPU IR late optimizations
; GCN-O3-NEXT: Basic Alias Analysis (stateless AA impl)
@@ -1161,7 +1136,6 @@
; GCN-O3-NEXT: Detect single entry single exit regions
; GCN-O3-NEXT: Region Pass Manager
; GCN-O3-NEXT: Structurize control flow
-; GCN-O3-NEXT: Cycle Info Analysis
; GCN-O3-NEXT: Uniformity Analysis
; GCN-O3-NEXT: Basic Alias Analysis (stateless AA impl)
; GCN-O3-NEXT: Function Alias Analysis Results
@@ -1169,7 +1143,6 @@
; GCN-O3-NEXT: AMDGPU Annotate Uniform Values
; GCN-O3-NEXT: Natural Loop Information
; GCN-O3-NEXT: SI annotate control flow
-; GCN-O3-NEXT: Cycle Info Analysis
; GCN-O3-NEXT: Uniformity Analysis
; GCN-O3-NEXT: AMDGPU Rewrite Undef for PHI
; GCN-O3-NEXT: LCSSA Verifier
@@ -1181,7 +1154,6 @@
; GCN-O3-NEXT: Safe Stack instrumentation pass
; GCN-O3-NEXT: Insert stack protectors
; GCN-O3-NEXT: Dominator Tree Construction
-; GCN-O3-NEXT: Cycle Info Analysis
; GCN-O3-NEXT: Uniformity Analysis
; GCN-O3-NEXT: Basic Alias Analysis (stateless AA impl)
; GCN-O3-NEXT: Function Alias Analysis Results
|
|
Similar symptoms with the old pass manager were previously seen elsewhere: |
jayfoad
reviewed
Dec 21, 2023
| // needed, and only affects the old pass manager. | ||
| m_cycleInfo.clear(); | ||
| m_cycleInfo.compute(F); | ||
| m_uniformityInfo = UniformityInfo{domTree, m_cycleInfo, &targetTransformInfo}; |
There was a problem hiding this comment.
The transitive dependency means that CycleInfo must be present and correct not just when UniformityInfo is run, but whenever it is queried. If UnifyDivergentExitNodes claims to preserve UniformityInfo but does not update CycleInfo then it sounds like there is a real problem there: as soon as UnifyDivergentExitNodes has made some changes, a query on UniformityInfo will no longer be reliable because CycleInfo will not be up to date. I don't see how computing CycleInfo in this "run" function fixes that problem.
There was a problem hiding this comment.
I always find transitive dependencies in the old pass manager confusing. It is supposed to mean what you said, but the actual implementation is not clear to me.
Specifically for UniformityInfo, CycleInfo is used only during its computation and does not need to be present during the lifetime of the UniformityInfo itself. But then there is this implicit assumption in general, if UniformityInfo is updated, then CycleInfo may need to be present as a dependency. This is not true in this case because UnifyDivergentExitNodes doesn't actually update UniformityInfo in any way. If all this is implemented correctly, then CycleInfo does not need to be marked transitive ... it is not needed to be alive during the lifetime of UniformityInfo.
Actually @Pierre-vh, you had marked CycleInfo as a transitive dependency at some point; it must have fixed something for you. Do you remember what that was?
There was a problem hiding this comment.
Oh wait, the query isTemporalDivergent() needs CycleInfo, during the lifetime of UniformityInfo. I'll need to rethink my approach here ... either claim that a local copy of CycleInfo is sufficient, or preserve UniformityInfo only when it makes sense to also preserve CycleInfo.
There was a problem hiding this comment.
I'd expect the same sort of CFG transforms to invalidate both, so intuitively to me it makes sense they would always be paired
|
#76696 is a simpler approach to fix this dependency: Until we have a way to preserve CycleInfo whenever we preserve UniformityInfo, we just don't try to preserve UniformityInfo. |
|
Superseded by #76696 |
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.
UniformityAnalysis depends on CycleInfo, but when a transform "preserves" on UniformityAnalysis, the transitive dependence can causes issues in the pass manager. The correct way to fix this would be to also update and preserve CycleInfo. But instead of that, we prefer to just recompute CycleInfo for UniformityAnalysis because the situation is rare enough.