[llvm] LAA: refactor analyzeLoop to return bool (NFC) (PR #93824)
Ramkumar Ramachandra via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 11 03:34:52 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/3] 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/3] 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;
}
>From 33c02cbbee7091dedefde6bcaa87bd1d543f06f8 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <r at artagnon.com>
Date: Tue, 11 Jun 2024 11:31:01 +0100
Subject: [PATCH 3/3] LAA: address review
---
llvm/lib/Analysis/LoopAccessAnalysis.cpp | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 3f51cef5f14ba..e36e890c8f867 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -2511,7 +2511,7 @@ bool LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
// Check if we see any stores. If there are no stores, then we don't
// care if the pointers are *restrict*.
- if (!NumStores) {
+ if (!Stores.size()) {
LLVM_DEBUG(dbgs() << "LAA: Found a read-only loop!\n");
return true;
}
@@ -2548,7 +2548,6 @@ bool LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
++NumReadWrites;
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.
@@ -2572,15 +2571,6 @@ bool LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
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
@@ -2597,7 +2587,14 @@ bool 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.
MemoryLocation Loc = MemoryLocation::get(LD);
+ 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;
+ }
// The TBAA metadata could have a control dependency on the predication
// condition, so we cannot rely on it when determining whether or not we
@@ -2612,8 +2609,8 @@ 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 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) {
LLVM_DEBUG(dbgs() << "LAA: Found a write-only loop!\n");
return true;
More information about the llvm-commits
mailing list