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

[Jay Foad via Phabricator via llvm-commits]

foad 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()) {
----------------
sameerds wrote:
> 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.
> so this function is simply a lookup in a set

It's implemented as a linear search through a std::vector.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146018