[PATCH] D149646: [LLVM][Uniformity] Propagate temporal divergence explicitly

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/include/llvm/ADT/GenericUniformityImpl.h:457-458
 
   /// \brief Mark all instruction as divergent that use a value defined in \p
   /// OuterDivCycle. Push their users on the worklist.
+  void propagateTemporalDivergence(const InstructionT &I,
----------------
Comment should be updated. It doesn't mention I.


================
Comment at: llvm/include/llvm/ADT/GenericUniformityImpl.h:824
     const CycleT &OuterDivCycle) {
-  // Set of blocks that are dominated by the cycle, i.e., each is only
-  // reachable from paths that pass through the cycle.
-  SmallPtrSet<BlockT *, 16> DomRegion;
-
-  // The boundary of DomRegion, formed by blocks that are not
-  // dominated by the cycle.
-  SmallVector<BlockT *> DomFrontier;
-  OuterDivCycle.getExitBlocks(DomFrontier);
-
-  // Returns true if BB is dominated by the cycle.
-  auto isInDomRegion = [&](BlockT *BB) {
-    for (auto *P : predecessors(BB)) {
-      if (OuterDivCycle.contains(P))
-        continue;
-      if (DomRegion.count(P))
-        continue;
-      return false;
-    }
-    return true;
-  };
-
-  // Keep advancing the frontier along successor edges, while
-  // promoting blocks to DomRegion.
-  while (true) {
-    bool Promoted = false;
-    SmallVector<BlockT *> Temp;
-    for (auto *W : DomFrontier) {
-      if (!isInDomRegion(W)) {
-        Temp.push_back(W);
-        continue;
-      }
-      DomRegion.insert(W);
-      Promoted = true;
-      for (auto *Succ : successors(W)) {
-        if (DomRegion.contains(Succ))
-          continue;
-        Temp.push_back(Succ);
-      }
-    }
-    if (!Promoted)
-      break;
-
-    // Restore the set property for the temporary vector
-    llvm::sort(Temp);
-    Temp.erase(std::unique(Temp.begin(), Temp.end()), Temp.end());
-
-    DomFrontier = Temp;
-  }
-
-  // At DomFrontier, only the PHI nodes are affected by temporal
-  // divergence.
-  for (const auto *UserBlock : DomFrontier) {
-    LLVM_DEBUG(dbgs() << "Analyze phis after cycle exit: "
-                      << Context.print(UserBlock) << "\n");
-    for (const auto &Phi : UserBlock->phis()) {
-      LLVM_DEBUG(dbgs() << "  " << Context.print(&Phi) << "\n");
-      analyzeTemporalDivergence(Phi, OuterDivCycle);
-    }
-  }
-
-  // All instructions inside the dominance region are affected by
-  // temporal divergence.
-  for (const auto *UserBlock : DomRegion) {
-    LLVM_DEBUG(dbgs() << "Analyze non-phi users after cycle exit: "
-                      << Context.print(UserBlock) << "\n");
-    for (const auto &I : *UserBlock) {
-      LLVM_DEBUG(dbgs() << "  " << Context.print(&I) << "\n");
-      analyzeTemporalDivergence(I, OuterDivCycle);
+  for (auto *BB : OuterDivCycle.blocks()) {
+    for (auto &II : *BB) {
----------------
I wonder if there is a quick test you could use to avoid scanning some BBs, e.g. I think if BB does not dominate any edges that exit OuterDivCycle then instructions in BB cannot have any uses outside OuterDivCycle. This should be fairly common because it applies to the body of a simple C "for" loop.


================
Comment at: llvm/lib/Analysis/UniformityAnalysis.cpp:72-73
+                                             const Cycle &DefCycle) {
+  for (const Use &U : I.uses()) {
+    auto *UserInstr = cast<Instruction>(U.getUser());
+    if (DefCycle.contains(UserInstr->getParent()))
----------------
Just iterate over `users()` instead of `uses()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149646