[llvm] [LAA] refactor program logic (NFC) (PR #92101)
Ramkumar Ramachandra via llvm-commits
llvm-commits at lists.llvm.org
Wed May 22 05:41:34 PDT 2024
https://github.com/artagnon updated https://github.com/llvm/llvm-project/pull/92101
>From f5026c3849064d425f61811c22fde2146455f0db Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <r at artagnon.com>
Date: Tue, 14 May 2024 11:55:51 +0100
Subject: [PATCH 1/2] [LAA] refactor program logic (NFC)
Implement NFC improvements spotted during a cursory reading of
LoopAccessAnalysis.
---
llvm/lib/Analysis/LoopAccessAnalysis.cpp | 105 ++++++++++-------------
1 file changed, 47 insertions(+), 58 deletions(-)
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index df01ad119a8be..33669966b4504 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -389,9 +389,9 @@ void RuntimePointerChecking::generateChecks(
bool RuntimePointerChecking::needsChecking(
const RuntimeCheckingPtrGroup &M, const RuntimeCheckingPtrGroup &N) const {
- for (unsigned I = 0, EI = M.Members.size(); EI != I; ++I)
- for (unsigned J = 0, EJ = N.Members.size(); EJ != J; ++J)
- if (needsChecking(M.Members[I], N.Members[J]))
+ for (auto &I : M.Members)
+ for (auto &J : N.Members)
+ if (needsChecking(I, J))
return true;
return false;
}
@@ -405,9 +405,7 @@ static const SCEV *getMinFromExprs(const SCEV *I, const SCEV *J,
if (!C)
return nullptr;
- if (C->getValue()->isNegative())
- return J;
- return I;
+ return C->getValue()->isNegative() ? J : I;
}
bool RuntimeCheckingPtrGroup::addPointer(unsigned Index,
@@ -505,8 +503,8 @@ void RuntimePointerChecking::groupChecks(
DenseMap<Value *, SmallVector<unsigned>> PositionMap;
for (unsigned Index = 0; Index < Pointers.size(); ++Index) {
- auto Iter = PositionMap.insert({Pointers[Index].PointerValue, {}});
- Iter.first->second.push_back(Index);
+ auto [It, _] = PositionMap.insert({Pointers[Index].PointerValue, {}});
+ It->second.push_back(Index);
}
// We need to keep track of what pointers we've already seen so we
@@ -605,16 +603,16 @@ void RuntimePointerChecking::printChecks(
raw_ostream &OS, const SmallVectorImpl<RuntimePointerCheck> &Checks,
unsigned Depth) const {
unsigned N = 0;
- for (const auto &Check : Checks) {
- const auto &First = Check.first->Members, &Second = Check.second->Members;
+ for (const auto &[Check1, Check2] : Checks) {
+ const auto &First = Check1->Members, &Second = Check2->Members;
OS.indent(Depth) << "Check " << N++ << ":\n";
- OS.indent(Depth + 2) << "Comparing group (" << Check.first << "):\n";
+ OS.indent(Depth + 2) << "Comparing group (" << Check1 << "):\n";
for (unsigned K = 0; K < First.size(); ++K)
OS.indent(Depth + 2) << *Pointers[First[K]].PointerValue << "\n";
- OS.indent(Depth + 2) << "Against group (" << Check.second << "):\n";
+ OS.indent(Depth + 2) << "Against group (" << Check2 << "):\n";
for (unsigned K = 0; K < Second.size(); ++K)
OS.indent(Depth + 2) << *Pointers[Second[K]].PointerValue << "\n";
}
@@ -1155,8 +1153,8 @@ bool AccessAnalysis::canCheckPtrAtRT(RuntimePointerChecking &RtCheck,
// First, count how many write and read accesses are in the alias set. Also
// collect MemAccessInfos for later.
SmallVector<MemAccessInfo, 4> AccessInfos;
- for (const Value *Ptr_ : ASPointers) {
- Value *Ptr = const_cast<Value *>(Ptr_);
+ for (const Value *ConstPtr : ASPointers) {
+ Value *Ptr = const_cast<Value *>(ConstPtr);
bool IsWrite = Accesses.count(MemAccessInfo(Ptr, true));
if (IsWrite)
++NumWritePtrChecks;
@@ -1212,9 +1210,7 @@ bool AccessAnalysis::canCheckPtrAtRT(RuntimePointerChecking &RtCheck,
// We know that we need these checks, so we can now be more aggressive
// and add further checks if required (overflow checks).
CanDoAliasSetRT = true;
- for (auto Retry : Retries) {
- MemAccessInfo Access = Retry.first;
- Type *AccessTy = Retry.second;
+ for (const auto &[Access, AccessTy] : Retries) {
if (!createCheckForAccess(RtCheck, Access, AccessTy, StridesMap,
DepSetId, TheLoop, RunningDepId, ASId,
ShouldCheckWrap, /*Assume=*/true)) {
@@ -1286,12 +1282,11 @@ void AccessAnalysis::processMemAccesses() {
LLVM_DEBUG(dbgs() << " AST: "; AST.dump());
LLVM_DEBUG(dbgs() << "LAA: Accesses(" << Accesses.size() << "):\n");
LLVM_DEBUG({
- for (auto A : Accesses)
- dbgs() << "\t" << *A.first.getPointer() << " ("
- << (A.first.getInt()
- ? "write"
- : (ReadOnlyPtr.count(A.first.getPointer()) ? "read-only"
- : "read"))
+ for (const auto &[A, _] : Accesses)
+ dbgs() << "\t" << *A.getPointer() << " ("
+ << (A.getInt() ? "write"
+ : (ReadOnlyPtr.count(A.getPointer()) ? "read-only"
+ : "read"))
<< ")\n";
});
@@ -1320,16 +1315,16 @@ void AccessAnalysis::processMemAccesses() {
bool UseDeferred = SetIteration > 0;
PtrAccessMap &S = UseDeferred ? DeferredAccesses : Accesses;
- for (const Value *Ptr_ : ASPointers) {
- Value *Ptr = const_cast<Value *>(Ptr_);
+ for (const Value *ConstPtr : ASPointers) {
+ Value *Ptr = const_cast<Value *>(ConstPtr);
// For a single memory access in AliasSetTracker, Accesses may contain
// both read and write, and they both need to be handled for CheckDeps.
- for (const auto &AC : S) {
- if (AC.first.getPointer() != Ptr)
+ for (const auto &[AC, _] : S) {
+ if (AC.getPointer() != Ptr)
continue;
- bool IsWrite = AC.first.getInt();
+ bool IsWrite = AC.getInt();
// If we're using the deferred access set, then it contains only
// reads.
@@ -1856,10 +1851,7 @@ static bool isSafeDependenceDistance(const DataLayout &DL, ScalarEvolution &SE,
// (If so, then we have proven (**) because |Dist| >= -1*Dist)
const SCEV *NegDist = SE.getNegativeSCEV(CastedDist);
Minus = SE.getMinusSCEV(NegDist, CastedProduct);
- if (SE.isKnownPositive(Minus))
- return true;
-
- return false;
+ return SE.isKnownPositive(Minus);
}
/// Check the dependence for two accesses with the same stride \p Stride.
@@ -2030,7 +2022,7 @@ MemoryDepChecker::Dependence::DepType MemoryDepChecker::isDependent(
if (isa<SCEVCouldNotCompute>(Dist)) {
// TODO: Relax requirement that there is a common stride to retry with
// non-constant distance dependencies.
- FoundNonConstantDistanceDependence |= !!CommonStride;
+ FoundNonConstantDistanceDependence |= CommonStride.has_value();
LLVM_DEBUG(dbgs() << "LAA: Dependence because of uncomputable distance.\n");
return Dependence::Unknown;
}
@@ -2070,14 +2062,12 @@ MemoryDepChecker::Dependence::DepType MemoryDepChecker::isDependent(
// Negative distances are not plausible dependencies.
if (SE.isKnownNonPositive(Dist)) {
if (SE.isKnownNonNegative(Dist)) {
- if (HasSameSize) {
+ if (HasSameSize)
// Write to the same location with the same size.
return Dependence::Forward;
- } else {
- LLVM_DEBUG(dbgs() << "LAA: possibly zero dependence difference but "
- "different type sizes\n");
- return Dependence::Unknown;
- }
+ LLVM_DEBUG(dbgs() << "LAA: possibly zero dependence difference but "
+ "different type sizes\n");
+ return Dependence::Unknown;
}
bool IsTrueDataDependence = (AIsWrite && !BIsWrite);
@@ -2323,7 +2313,7 @@ bool MemoryDepChecker::areDepsSafe(
}
++OI;
}
- AI++;
+ ++AI;
}
}
@@ -2332,8 +2322,8 @@ bool MemoryDepChecker::areDepsSafe(
}
SmallVector<Instruction *, 4>
-MemoryDepChecker::getInstructionsForAccess(Value *Ptr, bool isWrite) const {
- MemAccessInfo Access(Ptr, isWrite);
+MemoryDepChecker::getInstructionsForAccess(Value *Ptr, bool IsWrite) const {
+ MemAccessInfo Access(Ptr, IsWrite);
auto &IndexVector = Accesses.find(Access)->second;
SmallVector<Instruction *, 4> Insts;
@@ -2709,13 +2699,14 @@ void LoopAccessInfo::analyzeLoop(AAResults *AA, LoopInfo *LI,
}
void LoopAccessInfo::emitUnsafeDependenceRemark() {
- auto Deps = getDepChecker().getDependences();
+ const auto *Deps = getDepChecker().getDependences();
if (!Deps)
return;
- auto Found = llvm::find_if(*Deps, [](const MemoryDepChecker::Dependence &D) {
- return MemoryDepChecker::Dependence::isSafeForVectorization(D.Type) !=
- MemoryDepChecker::VectorizationSafetyStatus::Safe;
- });
+ const auto *Found =
+ llvm::find_if(*Deps, [](const MemoryDepChecker::Dependence &D) {
+ return MemoryDepChecker::Dependence::isSafeForVectorization(D.Type) !=
+ MemoryDepChecker::VectorizationSafetyStatus::Safe;
+ });
if (Found == Deps->end())
return;
MemoryDepChecker::Dependence Dep = *Found;
@@ -2854,9 +2845,9 @@ static Value *stripGetElementPtr(Value *Ptr, ScalarEvolution *SE, Loop *Lp) {
// Check that all of the gep indices are uniform except for our induction
// operand.
- for (unsigned i = 0, e = GEP->getNumOperands(); i != e; ++i)
- if (i != InductionOperand &&
- !SE->isLoopInvariant(SE->getSCEV(GEP->getOperand(i)), Lp))
+ for (unsigned I = 0, E = GEP->getNumOperands(); I != E; ++I)
+ if (I != InductionOperand &&
+ !SE->isLoopInvariant(SE->getSCEV(GEP->getOperand(I)), Lp))
return Ptr;
return GEP->getOperand(InductionOperand);
}
@@ -3038,11 +3029,10 @@ LoopAccessInfo::LoopAccessInfo(Loop *L, ScalarEvolution *SE,
if (TTI) {
TypeSize FixedWidth =
TTI->getRegisterBitWidth(TargetTransformInfo::RGK_FixedWidthVector);
- if (FixedWidth.isNonZero()) {
+ if (FixedWidth.isNonZero())
// Scale the vector width by 2 as rough estimate to also consider
// interleaving.
MaxTargetVectorWidthInBits = FixedWidth.getFixedValue() * 2;
- }
TypeSize ScalableWidth =
TTI->getRegisterBitWidth(TargetTransformInfo::RGK_ScalableVector);
@@ -3052,9 +3042,8 @@ LoopAccessInfo::LoopAccessInfo(Loop *L, ScalarEvolution *SE,
DepChecker =
std::make_unique<MemoryDepChecker>(*PSE, L, MaxTargetVectorWidthInBits);
PtrRtChecking = std::make_unique<RuntimePointerChecking>(*DepChecker, SE);
- if (canAnalyzeLoop()) {
+ if (canAnalyzeLoop())
analyzeLoop(AA, LI, TLI, DT);
- }
}
void LoopAccessInfo::print(raw_ostream &OS, unsigned Depth) const {
@@ -3106,13 +3095,13 @@ void LoopAccessInfo::print(raw_ostream &OS, unsigned Depth) const {
}
const LoopAccessInfo &LoopAccessInfoManager::getInfo(Loop &L) {
- auto I = LoopAccessInfoMap.insert({&L, nullptr});
+ auto [It, Inserted] = LoopAccessInfoMap.insert({&L, nullptr});
- if (I.second)
- I.first->second =
+ if (Inserted)
+ It->second =
std::make_unique<LoopAccessInfo>(&L, &SE, TTI, TLI, &AA, &DT, &LI);
- return *I.first->second;
+ return *It->second;
}
bool LoopAccessInfoManager::invalidate(
>From 493c15fd11a49943392670109fc0663897b9743f Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <r at artagnon.com>
Date: Wed, 22 May 2024 13:39:21 +0100
Subject: [PATCH 2/2] [LAA] address review (NFC)
---
llvm/lib/Analysis/LoopAccessAnalysis.cpp | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 33669966b4504..51c00cd0a7bb0 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -389,8 +389,8 @@ void RuntimePointerChecking::generateChecks(
bool RuntimePointerChecking::needsChecking(
const RuntimeCheckingPtrGroup &M, const RuntimeCheckingPtrGroup &N) const {
- for (auto &I : M.Members)
- for (auto &J : N.Members)
+ for (const auto &I : M.Members)
+ for (const auto &J : N.Members)
if (needsChecking(I, J))
return true;
return false;
@@ -2062,9 +2062,10 @@ MemoryDepChecker::Dependence::DepType MemoryDepChecker::isDependent(
// Negative distances are not plausible dependencies.
if (SE.isKnownNonPositive(Dist)) {
if (SE.isKnownNonNegative(Dist)) {
- if (HasSameSize)
+ if (HasSameSize) {
// Write to the same location with the same size.
return Dependence::Forward;
+ }
LLVM_DEBUG(dbgs() << "LAA: possibly zero dependence difference but "
"different type sizes\n");
return Dependence::Unknown;
@@ -3029,10 +3030,11 @@ LoopAccessInfo::LoopAccessInfo(Loop *L, ScalarEvolution *SE,
if (TTI) {
TypeSize FixedWidth =
TTI->getRegisterBitWidth(TargetTransformInfo::RGK_FixedWidthVector);
- if (FixedWidth.isNonZero())
+ if (FixedWidth.isNonZero()) {
// Scale the vector width by 2 as rough estimate to also consider
// interleaving.
MaxTargetVectorWidthInBits = FixedWidth.getFixedValue() * 2;
+ }
TypeSize ScalableWidth =
TTI->getRegisterBitWidth(TargetTransformInfo::RGK_ScalableVector);
More information about the llvm-commits
mailing list