[llvm] [LLVM][Transforms][Attributor] - Add batch reachability optimization to forallInterferingAccesses (PR #190078)

[Pranav Bhandarkar via llvm-commits]

================
@@ -1343,13 +1363,297 @@ struct AAPointerInfoImpl
       return LeastDominatingWriteInst != Acc.getRemoteInst();
     };
 
-    // Run the user callback on all accesses we cannot skip and return if
-    // that succeeded for all or not.
-    for (auto &It : InterferingAccesses) {
-      if ((!AllInSameNoSyncFn && !IsThreadLocalObj && !ExecDomainAA) ||
-          !CanSkipAccess(*It.first, It.second)) {
-        if (!UserCB(*It.first, It.second))
+    {
+      // Batch reachability optimization for CanSkipAccess.
+      //
+      // Without this optimization, each interfering access would trigger two
+      // independent AA::isPotentiallyReachable calls. Each call traverses the
+      // CFG from scratch. With N interfering accesses, this is 2 * N
+      // independent BFS traversals over the same function's CFG — redundant
+      // work.
+      //
+      // This optimization pre-computes two block-level reachability sets:
+      //   ReachableFromI: all basic blocks reachable forward from I's block
+      //   ReachableToI:   all basic blocks that can reach I's block (backward)
+      //
+      // These are computed lazily (on first use) via a single BFS each,
+      // respecting the ExclusionSet (must-write barriers) and liveness
+      // (dead edges from AAIsDead). Then for each intra-function access,
+      // a cheap set lookup replaces the full isPotentiallyReachable call:
+      //   - ReadChecked:  if Acc's block is NOT in ReachableFromI, I can't
+      //                   reach Acc, so Acc's read can't observe I's write.
+      //   - WriteChecked: if Acc's block is NOT in ReachableToI, Acc can't
+      //                   reach I, so Acc's write can't affect I's read.
+      //
+      // If the block-level check is inconclusive, we fall back to
+      // isPotentiallyReachable.
+      //
+      //
+
+      DenseSet<const BasicBlock *> ReachableFromI;
+      bool ReachableFromIComputed = false;
+
+      DenseSet<const BasicBlock *> ReachableToI;
+      bool ReachableToIComputed = false;
+
+      // Map each basic block to the ExclusionSet instructions it contains.
+      // Built once and shared across the BFS helpers and same-block checks
+      // in CanSkipAccessBatch, replacing per-use iteration over ExclusionSet.
+      DenseMap<const BasicBlock *, SmallVector<const Instruction *, 2>>
+          ExcludedBlockInsts;
+      for (const Instruction *ExclI : ExclusionSet)
+        ExcludedBlockInsts[ExclI->getParent()].push_back(ExclI);
+
+      // Lazily compute forward reachability from I's block.
+      // BFS over successor edges within Scope, skipping dead edges (AAIsDead)
+      // and not traversing past ExclusionSet blocks (must-write barriers).
+      // I's block is always traversed (its successors are always explored).
+      // Other blocks containing ExclusionSet instructions are added to the
+      // reachable set but their successors are NOT explored.
+      //
+      // Note: we do NOT block I's block even if it contains an ExclusionSet
+      // instruction after I. This matches isPotentiallyReachable /
+      // isReachableImpl semantics, which check SuccBB == ToBB before
+      // ExclusionBlocks and thus always consider direct successors as
----------------
bhandarkar-pranav wrote:

Good catch. The comment is misleading — the analogy to `isPotentiallyReachable`'s invariant doesn't actually hold for the BFS. `isPotentallyReachable` keeps the post-`SuccBB == ToBB`/pre-`ExclusionBlocks` ordering precise because at the top of `isReachableImpl` it pre-checks `WillReachInBlock(ToBB->front(), *RQI.To, ExclusionSet)` to handle the in-block barrier; the BFS here has no such per-Acc pre-check.

What I actually rely on is something different: the BFS is only used as a **negative filter** (we short-circuit only when the BFS says "not reachable"). If a barrier inside Acc's block makes `isPotentiallyReachable` say "not reachable" but the BFS says "reachable", that's a missed fast-path, not a correctness bug — we fall through to `isPotentiallyReachable` and pay the cost of the call. So leaving exclusion-set blocks in the reachable set (but not exploring past them) is an over-approximation.

I'll rewrite the comment to say that explicitly. If we  want to recover precision, we can scan `ExclusionSet` instructions in `Acc.getRemoteInst()->getParent()` (using the already-built
t `ExcludedBlockInsts` map) to see if any is between the block entry and `Acc.getRemoteInst()`. Happy to add that as a small follow-up if you'd like; for now I'd prefer to keep this patch focused.


https://github.com/llvm/llvm-project/pull/190078