[PATCH] D146018: [AMDGPU] Use UniformityAnalysis in AtomicOptimizer

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds added inline comments.


================
Comment at: llvm/include/llvm/ADT/GenericUniformityImpl.h:1102
+  for (const CycleT *Cycle = CI.getCycle(DefBlock);
+       Cycle && !Cycle->contains(&ObservingBlock);
+       Cycle = Cycle->getParentCycle()) {
----------------
Pierre-vh wrote:
> foad wrote:
> > Nit: the Cycle->contains check seems like it could get expensive on pathological cases with thousands of nested loops. Would it be more efficient to walk up the cycle tree from `getCycle(Def)` to `nearestCommonAncestor(getCycle(Def), getCycle(Use))`?
> I'm not too familiar with CycleInfo so I'll leave this one to @sameerds
Every cycle contains a list of all the blocks that it transitively contains, so this function is simply a lookup in a set. This is the same behaviour as LoopInfo.

But now that nested cycles got mentioned, I think this change should have another test with a def inside the inner of two nested cycles, and a use of that def outside the outer cycle.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146018/new/

https://reviews.llvm.org/D146018