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

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added inline comments.


================
Comment at: llvm/include/llvm/ADT/GenericUniformityImpl.h:466
-  /// \brief Whether \p Val is divergent when read in \p ObservingBlock.
+  /// \brief Whether \p Def is divergent when read in \p ObservingBlock.
   bool isTemporalDivergent(const BlockT &ObservingBlock,
-                           ConstValueRefT Val) const;
----------------
foad wrote:
> Was this declaration unused?
Yes, it had no definition for UA beforehand.


================
Comment at: llvm/include/llvm/ADT/GenericUniformityImpl.h:1102
+  for (const CycleT *Cycle = CI.getCycle(DefBlock);
+       Cycle && !Cycle->contains(&ObservingBlock);
+       Cycle = Cycle->getParentCycle()) {
----------------
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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146018