[AMDGPU] Do not preserve UniformityInfo

[ssahasra]

UniformityInfo has a transitive dependence on CycleInfo. A transform may change the CFG in trivial ways that do not affect uniformity, but that can leave cycles in a slightly inconsistent state. In the absence of updates to CycleInfo, it's cleaner to just invalidate both analyses.

This supersedes #76011, which tries to recompute CycleInfo when computing UniformityInfo.

[llvmbot]

@llvm/pr-subscribers-llvm-adt

@llvm/pr-subscribers-backend-amdgpu

Author: Sameer Sahasrabuddhe (ssahasra)

Changes

UniformityInfo has a transitive dependence on CycleInfo. A transform may change the CFG in trivial ways that do not affect uniformity, but that can leave cycles in a slightly inconsistent state. In the absence of updates to CycleInfo, it's cleaner to just invalidate both analyses.

This supersedes #76011, which tries to recompute CycleInfo when computing UniformityInfo.

---

Full diff: https://github.com/llvm/llvm-project/pull/76696.diff

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/AMDGPURewriteUndefForPHI.cpp (-1)
• (modified) llvm/lib/Target/AMDGPU/AMDGPUUnifyDivergentExitNodes.cpp (-3)

diff --git a/llvm/lib/Target/AMDGPU/AMDGPURewriteUndefForPHI.cpp b/llvm/lib/Target/AMDGPU/AMDGPURewriteUndefForPHI.cpp
index 459400e3359ca1..79e9312034da45 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPURewriteUndefForPHI.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPURewriteUndefForPHI.cpp
@@ -85,7 +85,6 @@ class AMDGPURewriteUndefForPHILegacy : public FunctionPass {
     AU.addRequired<DominatorTreeWrapperPass>();
 
     AU.addPreserved<DominatorTreeWrapperPass>();
-    AU.addPreserved<UniformityInfoWrapperPass>();
     AU.setPreservesCFG();
   }
 };
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUUnifyDivergentExitNodes.cpp b/llvm/lib/Target/AMDGPU/AMDGPUUnifyDivergentExitNodes.cpp
index 9bc3ba161c9ebe..1bfb7c0edd80a9 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUUnifyDivergentExitNodes.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUUnifyDivergentExitNodes.cpp
@@ -109,9 +109,6 @@ void AMDGPUUnifyDivergentExitNodes::getAnalysisUsage(AnalysisUsage &AU) const {
     // FIXME: preserve PostDominatorTreeWrapperPass
   }
 
-  // No divergent values are changed, only blocks and branch edges.
-  AU.addPreserved<UniformityInfoWrapperPass>();
-
   // We preserve the non-critical-edgeness property
   AU.addPreservedID(BreakCriticalEdgesID);
 

[arsenm]

Probably should add an explanatory comment

[ssahasra]

What would be the value of such a comment tucked away in some pass? Why only these specific passes? Why not others? We are just returning to the original state that we don't really know how to preserve UniformityAnalysis in general.

[arsenm]

Mostly I'm worried about someone getting the idea again that this could be preserved. Feels like it should be documented somewhere, but there isn't a good place. Will anything break in a future change if this is re-added?

[jayfoad]

Will anything break in a future change if this is re-added?

Unfortunately we have never had a good way to diagnose this problem (preserving an analysis but not one of its transitive dependencies). I did once look at adding something to the legacy pass manager to detect it early (when adding passes instead of when running them) but never finished it.

[arsenm]

Ditto

[arsenm]

"updation" should be updating?

[jayfoad]

"updation" should be a word! :-)

[ssahasra]

I had looked it up ... "updation" is a word, and it's use as a noun is not wrong here. "updating" requires an object after it. But it was not clear to me if the verb "updating" is preferred over the noun "updation" in general ... so went with @arsenm's preference, writing it off as an ESL thing, and/or a tussle between en_US, en_UK and en_IN :)