[llvm] LAA: refactor analyzeLoop to return bool (NFC) (PR #93824)

Ramkumar Ramachandra via llvm-commits llvm-commits at lists.llvm.org
Thu May 30 07:16:14 PDT 2024


https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/93824

>From 5cb7ceed28a92195f39030f662efb8feda5b9335 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <r at artagnon.com>
Date: Thu, 30 May 2024 11:22:56 +0100
Subject: [PATCH 1/2] LAA: refactor analyzeLoop to return bool (NFC)

Avoid wastefully setting CanVecMem in several places in analyzeLoop,
complicating the logic, to get the function to return a bool, and set
CanVecMem in the caller. While at it, also notice that the variable name
NumReadWrites is misleading, as it records the number of writes; change
the variable name to NumWrites for clarity.
---
 .../llvm/Analysis/LoopAccessAnalysis.h        |  7 +-
 llvm/lib/Analysis/LoopAccessAnalysis.cpp      | 92 +++++++++----------
 2 files changed, 47 insertions(+), 52 deletions(-)

diff --git a/llvm/include/llvm/Analysis/LoopAccessAnalysis.h b/llvm/include/llvm/Analysis/LoopAccessAnalysis.h
index b9f385f4c4b8f..453d12bda27eb 100644
--- a/llvm/include/llvm/Analysis/LoopAccessAnalysis.h
+++ b/llvm/include/llvm/Analysis/LoopAccessAnalysis.h
@@ -704,9 +704,10 @@ class LoopAccessInfo {
   const PredicatedScalarEvolution &getPSE() const { return *PSE; }
 
 private:
-  /// Analyze the loop.
-  void analyzeLoop(AAResults *AA, LoopInfo *LI,
-                   const TargetLibraryInfo *TLI, DominatorTree *DT);
+  /// Analyze the loop. Return a boolean indicating whether the memory can be
+  /// vectorized.
+  bool analyzeLoop(AAResults *AA, LoopInfo *LI, const TargetLibraryInfo *TLI,
+                   DominatorTree *DT);
 
   /// Check if the structure of the loop allows it to be analyzed by this
   /// pass.
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 13005cb8335d1..84a69a3c8c968 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -2386,7 +2386,7 @@ bool LoopAccessInfo::canAnalyzeLoop() {
   return true;
 }
 
-void LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
+bool LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
                                  const TargetLibraryInfo *TLI,
                                  DominatorTree *DT) {
   // Holds the Load and Store instructions.
@@ -2396,7 +2396,7 @@ void LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
 
   // Holds all the different accesses in the loop.
   unsigned NumReads = 0;
-  unsigned NumReadWrites = 0;
+  unsigned NumWrites = 0;
 
   bool HasComplexMemInst = false;
 
@@ -2427,10 +2427,8 @@ void LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
 
       // With both a non-vectorizable memory instruction and a convergent
       // operation, found in this loop, no reason to continue the search.
-      if (HasComplexMemInst && HasConvergentOp) {
-        CanVecMem = false;
-        return;
-      }
+      if (HasComplexMemInst && HasConvergentOp)
+        return false;
 
       // Avoid hitting recordAnalysis multiple times.
       if (HasComplexMemInst)
@@ -2505,20 +2503,17 @@ void LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
     } // Next instr.
   } // Next block.
 
-  if (HasComplexMemInst) {
-    CanVecMem = false;
-    return;
-  }
+  if (HasComplexMemInst)
+    return false;
 
   // Now we have two lists that hold the loads and the stores.
   // Next, we find the pointers that they use.
 
   // Check if we see any stores. If there are no stores, then we don't
   // care if the pointers are *restrict*.
-  if (!Stores.size()) {
+  if (!NumStores) {
     LLVM_DEBUG(dbgs() << "LAA: Found a read-only loop!\n");
-    CanVecMem = true;
-    return;
+    return true;
   }
 
   MemoryDepChecker::DepCandidates DependentAccesses;
@@ -2546,13 +2541,13 @@ void LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
           !UniformStores.insert(Ptr).second;
     }
 
-    // If we did *not* see this pointer before, insert it to  the read-write
-    // list. At this phase it is only a 'write' list.
+    // If we did *not* see this pointer before, insert it to  the write list.
     Type *AccessTy = getLoadStoreType(ST);
     if (Seen.insert({Ptr, AccessTy}).second) {
-      ++NumReadWrites;
+      ++NumWrites;
 
       MemoryLocation Loc = MemoryLocation::get(ST);
+
       // The TBAA metadata could have a control dependency on the predication
       // condition, so we cannot rely on it when determining whether or not we
       // need runtime pointer checks.
@@ -2571,12 +2566,20 @@ void LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
     LLVM_DEBUG(
         dbgs() << "LAA: A loop annotated parallel, ignore memory dependency "
                << "checks.\n");
-    CanVecMem = true;
-    return;
+    return true;
   }
 
   for (LoadInst *LD : Loads) {
     Value *Ptr = LD->getPointerOperand();
+
+    // See if there is an unsafe dependency between a load to a uniform address
+    // and store to the same uniform address.
+    if (UniformStores.count(Ptr)) {
+      LLVM_DEBUG(dbgs() << "LAA: Found an unsafe dependency between a uniform "
+                           "load and uniform store to the same address!\n");
+      HasLoadStoreDependenceInvolvingLoopInvariantAddress = true;
+    }
+
     // If we did *not* see this pointer before, insert it to the
     // read list. If we *did* see it before, then it is already in
     // the read-write list. This allows us to vectorize expressions
@@ -2593,15 +2596,8 @@ void LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
       IsReadOnlyPtr = true;
     }
 
-    // See if there is an unsafe dependency between a load to a uniform address and
-    // store to the same uniform address.
-    if (UniformStores.count(Ptr)) {
-      LLVM_DEBUG(dbgs() << "LAA: Found an unsafe dependency between a uniform "
-                           "load and uniform store to the same address!\n");
-      HasLoadStoreDependenceInvolvingLoopInvariantAddress = true;
-    }
-
     MemoryLocation Loc = MemoryLocation::get(LD);
+
     // The TBAA metadata could have a control dependency on the predication
     // condition, so we cannot rely on it when determining whether or not we
     // need runtime pointer checks.
@@ -2615,12 +2611,11 @@ void LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
                   });
   }
 
-  // If we write (or read-write) to a single destination and there are no
-  // other reads in this loop then is it safe to vectorize.
-  if (NumReadWrites == 1 && NumReads == 0) {
+  // If we write to a single destination and there are no other reads in this
+  // loop then is it safe to vectorize.
+  if (NumWrites == 1 && NumReads == 0) {
     LLVM_DEBUG(dbgs() << "LAA: Found a write-only loop!\n");
-    CanVecMem = true;
-    return;
+    return true;
   }
 
   // Build dependence sets and check whether we need a runtime pointer bounds
@@ -2639,21 +2634,20 @@ void LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
         << "cannot identify array bounds";
     LLVM_DEBUG(dbgs() << "LAA: We can't vectorize because we can't find "
                       << "the array bounds.\n");
-    CanVecMem = false;
-    return;
+    return false;
   }
 
   LLVM_DEBUG(
     dbgs() << "LAA: May be able to perform a memory runtime check if needed.\n");
 
-  CanVecMem = true;
+  bool DepsAreSafe = true;
   if (Accesses.isDependencyCheckNeeded()) {
     LLVM_DEBUG(dbgs() << "LAA: Checking memory dependencies\n");
-    CanVecMem = DepChecker->areDepsSafe(DependentAccesses,
-                                        Accesses.getDependenciesToCheck(),
-                                        Accesses.getUnderlyingObjects());
+    DepsAreSafe = DepChecker->areDepsSafe(DependentAccesses,
+                                          Accesses.getDependenciesToCheck(),
+                                          Accesses.getUnderlyingObjects());
 
-    if (!CanVecMem && DepChecker->shouldRetryWithRuntimeCheck()) {
+    if (!DepsAreSafe && DepChecker->shouldRetryWithRuntimeCheck()) {
       LLVM_DEBUG(dbgs() << "LAA: Retrying with memory checks\n");
 
       // Clear the dependency checks. We assume they are not needed.
@@ -2673,30 +2667,30 @@ void LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
         recordAnalysis("CantCheckMemDepsAtRunTime", I)
             << "cannot check memory dependencies at runtime";
         LLVM_DEBUG(dbgs() << "LAA: Can't vectorize with memory checks\n");
-        CanVecMem = false;
-        return;
+        return false;
       }
-
-      CanVecMem = true;
+      DepsAreSafe = true;
     }
   }
 
   if (HasConvergentOp) {
     recordAnalysis("CantInsertRuntimeCheckWithConvergent")
-      << "cannot add control dependency to convergent operation";
+        << "cannot add control dependency to convergent operation";
     LLVM_DEBUG(dbgs() << "LAA: We can't vectorize because a runtime check "
                          "would be needed with a convergent operation\n");
-    CanVecMem = false;
-    return;
+    return false;
   }
 
-  if (CanVecMem)
+  if (DepsAreSafe) {
     LLVM_DEBUG(
         dbgs() << "LAA: No unsafe dependent memory operations in loop.  We"
                << (PtrRtChecking->Need ? "" : " don't")
                << " need runtime memory checks.\n");
-  else
-    emitUnsafeDependenceRemark();
+    return true;
+  }
+
+  emitUnsafeDependenceRemark();
+  return false;
 }
 
 void LoopAccessInfo::emitUnsafeDependenceRemark() {
@@ -3045,7 +3039,7 @@ LoopAccessInfo::LoopAccessInfo(Loop *L, ScalarEvolution *SE,
                                                   MaxTargetVectorWidthInBits);
   PtrRtChecking = std::make_unique<RuntimePointerChecking>(*DepChecker, SE);
   if (canAnalyzeLoop())
-    analyzeLoop(AA, LI, TLI, DT);
+    CanVecMem = analyzeLoop(AA, LI, TLI, DT);
 }
 
 void LoopAccessInfo::print(raw_ostream &OS, unsigned Depth) const {

>From 593b6a685b0fd058cef76cf81097648c534c56c8 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <r at artagnon.com>
Date: Thu, 30 May 2024 15:13:05 +0100
Subject: [PATCH 2/2] LAA: fix thinko

---
 llvm/lib/Analysis/LoopAccessAnalysis.cpp | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 84a69a3c8c968..3f51cef5f14ba 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -2396,7 +2396,7 @@ bool LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
 
   // Holds all the different accesses in the loop.
   unsigned NumReads = 0;
-  unsigned NumWrites = 0;
+  unsigned NumReadWrites = 0;
 
   bool HasComplexMemInst = false;
 
@@ -2541,10 +2541,11 @@ bool LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
           !UniformStores.insert(Ptr).second;
     }
 
-    // If we did *not* see this pointer before, insert it to  the write list.
+    // If we did *not* see this pointer before, insert it to  the read-write
+    // list. At this phase it is only a 'write' list.
     Type *AccessTy = getLoadStoreType(ST);
     if (Seen.insert({Ptr, AccessTy}).second) {
-      ++NumWrites;
+      ++NumReadWrites;
 
       MemoryLocation Loc = MemoryLocation::get(ST);
 
@@ -2613,7 +2614,7 @@ bool LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
 
   // If we write to a single destination and there are no other reads in this
   // loop then is it safe to vectorize.
-  if (NumWrites == 1 && NumReads == 0) {
+  if (NumReadWrites == 1 && NumReads == 0) {
     LLVM_DEBUG(dbgs() << "LAA: Found a write-only loop!\n");
     return true;
   }



More information about the llvm-commits mailing list