[llvm] [BasicAA] Fix handling of indirect assumption based results (PR #100130)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 25 00:23:24 PDT 2024
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/100130
>From a8fbbbd94b1cce0416c7e5c0a919ae2076f99caf Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Tue, 23 Jul 2024 16:24:41 +0200
Subject: [PATCH 1/3] [SLP] Precommit test for #98978 (NFC)
---
.../Transforms/SLPVectorizer/X86/pr98978.ll | 99 +++++++++++++++++++
1 file changed, 99 insertions(+)
create mode 100644 llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll
diff --git a/llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll b/llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll
new file mode 100644
index 0000000000000..40e0db9d2f9a2
--- /dev/null
+++ b/llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll
@@ -0,0 +1,99 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=slp-vectorizer < %s | FileCheck %s
+
+target triple = "x86_64-redhat-linux-gnu"
+
+define void @test(ptr %p1, i64 %arg1, i64 %arg2) {
+; CHECK-LABEL: define void @test(
+; CHECK-SAME: ptr [[P1:%.*]], i64 [[ARG1:%.*]], i64 [[ARG2:%.*]]) {
+; CHECK-NEXT: [[ENTRY:.*]]:
+; CHECK-NEXT: br label %[[LOOP1:.*]]
+; CHECK: [[LOOP1]]:
+; CHECK-NEXT: [[I:%.*]] = phi ptr [ [[I21:%.*]], %[[BB20:.*]] ], [ [[P1]], %[[ENTRY]] ]
+; CHECK-NEXT: br i1 false, label %[[BB22:.*]], label %[[DOTPREHEADER48_PREHEADER_1:.*]]
+; CHECK: [[DEAD:.*]]:
+; CHECK-NEXT: br label %[[DOTPREHEADER48_PREHEADER_1]]
+; CHECK: [[_PREHEADER48_PREHEADER_1:.*:]]
+; CHECK-NEXT: [[I5:%.*]] = phi ptr [ [[I]], %[[DEAD]] ], [ [[I]], %[[LOOP1]] ]
+; CHECK-NEXT: br label %[[DOTLOOPEXIT49_1:.*]]
+; CHECK: [[DEAD1:.*]]:
+; CHECK-NEXT: br i1 false, label %[[DOTLOOPEXIT49_1]], label %[[BB20]]
+; CHECK: [[_LOOPEXIT49_1:.*:]]
+; CHECK-NEXT: [[I6:%.*]] = phi ptr [ [[I5]], %[[DEAD1]] ], [ [[I5]], %[[DOTPREHEADER48_PREHEADER_1]] ]
+; CHECK-NEXT: [[I7:%.*]] = getelementptr i8, ptr [[I6]], i64 [[ARG1]]
+; CHECK-NEXT: br label %[[BB10:.*]]
+; CHECK: [[DEAD2:.*]]:
+; CHECK-NEXT: br label %[[BB10]]
+; CHECK: [[BB10]]:
+; CHECK-NEXT: [[I11:%.*]] = phi ptr [ [[I7]], %[[DOTLOOPEXIT49_1]] ], [ null, %[[DEAD2]] ]
+; CHECK-NEXT: [[TMP0:%.*]] = load <2 x i64>, ptr [[I11]], align 1
+; CHECK-NEXT: [[TMP1:%.*]] = shufflevector <2 x i64> [[TMP0]], <2 x i64> poison, <2 x i32> <i32 1, i32 0>
+; CHECK-NEXT: store <2 x i64> [[TMP1]], ptr [[I6]], align 1
+; CHECK-NEXT: br label %[[BB20]]
+; CHECK: [[BB20]]:
+; CHECK-NEXT: [[I21]] = phi ptr [ [[I5]], %[[DEAD1]] ], [ [[I6]], %[[BB10]] ]
+; CHECK-NEXT: br label %[[LOOP1]]
+; CHECK: [[BB22]]:
+; CHECK-NEXT: [[I23:%.*]] = getelementptr i8, ptr [[I]], i64 [[ARG2]]
+; CHECK-NEXT: [[I25:%.*]] = getelementptr i8, ptr [[I23]], i64 8
+; CHECK-NEXT: br label %[[BB26:.*]]
+; CHECK: [[BB26]]:
+; CHECK-NEXT: [[I27:%.*]] = phi ptr [ null, %[[BB26]] ], [ [[I25]], %[[BB22]] ]
+; CHECK-NEXT: store i64 0, ptr [[I27]], align 1
+; CHECK-NEXT: [[I28:%.*]] = getelementptr i8, ptr [[I27]], i64 8
+; CHECK-NEXT: [[I29:%.*]] = load i64, ptr [[I23]], align 1
+; CHECK-NEXT: store i64 0, ptr [[I28]], align 1
+; CHECK-NEXT: br label %[[BB26]]
+;
+entry:
+ br label %loop1
+
+loop1: ; preds = %bb20, %entry
+ %i = phi ptr [ %i21, %bb20 ], [ %p1, %entry ]
+ br i1 false, label %bb22, label %.preheader48.preheader.1
+
+dead: ; No predecessors!
+ br label %.preheader48.preheader.1
+
+.preheader48.preheader.1: ; preds = %dead, %loop1
+ %i5 = phi ptr [ %i, %dead ], [ %i, %loop1 ]
+ br label %.loopexit49.1
+
+dead1: ; No predecessors!
+ br i1 false, label %.loopexit49.1, label %bb20
+
+.loopexit49.1: ; preds = %dead1, %.preheader48.preheader.1
+ %i6 = phi ptr [ %i5, %dead1 ], [ %i5, %.preheader48.preheader.1 ]
+ %i7 = getelementptr i8, ptr %i6, i64 %arg1
+ br label %bb10
+
+dead2: ; No predecessors!
+ br label %bb10
+
+bb10: ; preds = %dead2, %.loopexit49.1
+ %i11 = phi ptr [ %i7, %.loopexit49.1 ], [ null, %dead2 ]
+ %i16 = getelementptr i8, ptr %i11, i64 8
+ %i17 = load i64, ptr %i16, align 1
+ store i64 %i17, ptr %i6, align 1
+ %i18 = getelementptr i8, ptr %i6, i64 8
+ %i19 = load i64, ptr %i11, align 1
+ store i64 %i19, ptr %i18, align 1
+ br label %bb20
+
+bb20: ; preds = %bb10, %dead1
+ %i21 = phi ptr [ %i5, %dead1 ], [ %i6, %bb10 ]
+ br label %loop1
+
+bb22: ; preds = %loop1
+ %i23 = getelementptr i8, ptr %i, i64 %arg2
+ %i25 = getelementptr i8, ptr %i23, i64 8
+ br label %bb26
+
+bb26: ; preds = %bb26, %bb22
+ %i27 = phi ptr [ null, %bb26 ], [ %i25, %bb22 ]
+ store i64 0, ptr %i27, align 1
+ %i28 = getelementptr i8, ptr %i27, i64 8
+ %i29 = load i64, ptr %i23, align 1
+ store i64 0, ptr %i28, align 1
+ br label %bb26
+}
>From 386918ba56e6a3783af92aa165827e2ef7670308 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 24 Jul 2024 14:57:57 +0200
Subject: [PATCH 2/3] [BasicAA] Fix handling of indirect assumption based
results
If a result is potentially based on a not yet proven assumption,
BasicAA will remember it inside AssumptionBasedResults and remove
the cache entry it an assumption higher up is later disproven.
However, we currently miss the case where another cache entry ends
up depending on such an AssumptionBased result.
Fix this by introducing an additional AssumptionBased state for
cache entries. If such a result is used, we'll still increment
AAQI.NumAssumptionUses, which means that the using entry will
also become AssumptionBased and be cleared if the assumption is
disproven.
At the end of the root query, convert remaining AssumptionBased
results into definitive results.
Fixes https://github.com/llvm/llvm-project/issues/98978.
---
llvm/include/llvm/Analysis/AliasAnalysis.h | 17 +++++++++--
llvm/lib/Analysis/BasicAliasAnalysis.cpp | 28 ++++++++++++++++---
.../Transforms/SLPVectorizer/X86/pr98978.ll | 26 +++++++++--------
3 files changed, 53 insertions(+), 18 deletions(-)
diff --git a/llvm/include/llvm/Analysis/AliasAnalysis.h b/llvm/include/llvm/Analysis/AliasAnalysis.h
index 812b5a9f72a3a..4140387a1f341 100644
--- a/llvm/include/llvm/Analysis/AliasAnalysis.h
+++ b/llvm/include/llvm/Analysis/AliasAnalysis.h
@@ -244,12 +244,23 @@ class AAQueryInfo {
public:
using LocPair = std::pair<AACacheLoc, AACacheLoc>;
struct CacheEntry {
+ /// Cache entry is neither an assumption nor does it use a (non-definitive)
+ /// assumption.
+ static constexpr int Definitive = -2;
+ /// Cache entry is not an assumption itself, but may be using an assumption
+ /// from higher up the stack.
+ static constexpr int AssumptionBased = -1;
+
AliasResult Result;
- /// Number of times a NoAlias assumption has been used.
- /// 0 for assumptions that have not been used, -1 for definitive results.
+ /// Number of times a NoAlias assumption has been used, 0 for assumptions
+ /// that have not been used. Can also take one of the Definitive or
+ /// AssumptionBased values documented above.
int NumAssumptionUses;
+
/// Whether this is a definitive (non-assumption) result.
- bool isDefinitive() const { return NumAssumptionUses < 0; }
+ bool isDefinitive() const { return NumAssumptionUses == Definitive; }
+ /// Whether this is an assumption that has not been proven yet.
+ bool isAssumption() const { return NumAssumptionUses >= 0; }
};
// Alias analysis result aggregration using which this query is performed.
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index 161a3034e4829..e474899fb548e 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1692,9 +1692,12 @@ AliasResult BasicAAResult::aliasCheck(const Value *V1, LocationSize V1Size,
if (!Pair.second) {
auto &Entry = Pair.first->second;
if (!Entry.isDefinitive()) {
- // Remember that we used an assumption.
- ++Entry.NumAssumptionUses;
+ // Remember that we used an assumption. This may either be a direct use
+ // of an assumption, or a use of an entry that may itself be based on an
+ // assumption.
++AAQI.NumAssumptionUses;
+ if (Entry.isAssumption())
+ ++Entry.NumAssumptionUses;
}
// Cache contains sorted {V1,V2} pairs but we should return original order.
auto Result = Entry.Result;
@@ -1722,7 +1725,6 @@ AliasResult BasicAAResult::aliasCheck(const Value *V1, LocationSize V1Size,
Entry.Result = Result;
// Cache contains sorted {V1,V2} pairs.
Entry.Result.swap(Swapped);
- Entry.NumAssumptionUses = -1;
// If the assumption has been disproven, remove any results that may have
// been based on this assumption. Do this after the Entry updates above to
@@ -1734,8 +1736,26 @@ AliasResult BasicAAResult::aliasCheck(const Value *V1, LocationSize V1Size,
// The result may still be based on assumptions higher up in the chain.
// Remember it, so it can be purged from the cache later.
if (OrigNumAssumptionUses != AAQI.NumAssumptionUses &&
- Result != AliasResult::MayAlias)
+ Result != AliasResult::MayAlias) {
AAQI.AssumptionBasedResults.push_back(Locs);
+ Entry.NumAssumptionUses = AAQueryInfo::CacheEntry::AssumptionBased;
+ } else {
+ Entry.NumAssumptionUses = AAQueryInfo::CacheEntry::Definitive;
+ }
+
+ // Depth is incremented before this function is called, so Depth==1 indicates
+ // a root query.
+ if (AAQI.Depth == 1) {
+ // Any remaining assumption based results must be based on proven
+ // assumptions, so convert them to definitive results.
+ for (const auto &Loc : AAQI.AssumptionBasedResults) {
+ auto It = AAQI.AliasCache.find(Loc);
+ if (It != AAQI.AliasCache.end())
+ It->second.NumAssumptionUses = AAQueryInfo::CacheEntry::Definitive;
+ }
+ AAQI.AssumptionBasedResults.clear();
+ AAQI.NumAssumptionUses = 0;
+ }
return Result;
}
diff --git a/llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll b/llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll
index 40e0db9d2f9a2..3d14d286eff4f 100644
--- a/llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll
+++ b/llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll
@@ -3,22 +3,23 @@
target triple = "x86_64-redhat-linux-gnu"
+; Should not get vectorized.
define void @test(ptr %p1, i64 %arg1, i64 %arg2) {
; CHECK-LABEL: define void @test(
; CHECK-SAME: ptr [[P1:%.*]], i64 [[ARG1:%.*]], i64 [[ARG2:%.*]]) {
-; CHECK-NEXT: [[ENTRY:.*]]:
-; CHECK-NEXT: br label %[[LOOP1:.*]]
-; CHECK: [[LOOP1]]:
-; CHECK-NEXT: [[I:%.*]] = phi ptr [ [[I21:%.*]], %[[BB20:.*]] ], [ [[P1]], %[[ENTRY]] ]
+; CHECK-NEXT: [[_PREHEADER48_PREHEADER_1:.*]]:
+; CHECK-NEXT: br label %[[_LOOPEXIT49_1:.*]]
+; CHECK: [[_LOOPEXIT49_1]]:
+; CHECK-NEXT: [[I:%.*]] = phi ptr [ [[I21:%.*]], %[[BB20:.*]] ], [ [[P1]], %[[_PREHEADER48_PREHEADER_1]] ]
; CHECK-NEXT: br i1 false, label %[[BB22:.*]], label %[[DOTPREHEADER48_PREHEADER_1:.*]]
; CHECK: [[DEAD:.*]]:
; CHECK-NEXT: br label %[[DOTPREHEADER48_PREHEADER_1]]
-; CHECK: [[_PREHEADER48_PREHEADER_1:.*:]]
-; CHECK-NEXT: [[I5:%.*]] = phi ptr [ [[I]], %[[DEAD]] ], [ [[I]], %[[LOOP1]] ]
+; CHECK: [[_PREHEADER48_PREHEADER_2:.*:]]
+; CHECK-NEXT: [[I5:%.*]] = phi ptr [ [[I]], %[[DEAD]] ], [ [[I]], %[[_LOOPEXIT49_1]] ]
; CHECK-NEXT: br label %[[DOTLOOPEXIT49_1:.*]]
; CHECK: [[DEAD1:.*]]:
; CHECK-NEXT: br i1 false, label %[[DOTLOOPEXIT49_1]], label %[[BB20]]
-; CHECK: [[_LOOPEXIT49_1:.*:]]
+; CHECK: [[_LOOPEXIT49_2:.*:]]
; CHECK-NEXT: [[I6:%.*]] = phi ptr [ [[I5]], %[[DEAD1]] ], [ [[I5]], %[[DOTPREHEADER48_PREHEADER_1]] ]
; CHECK-NEXT: [[I7:%.*]] = getelementptr i8, ptr [[I6]], i64 [[ARG1]]
; CHECK-NEXT: br label %[[BB10:.*]]
@@ -26,13 +27,16 @@ define void @test(ptr %p1, i64 %arg1, i64 %arg2) {
; CHECK-NEXT: br label %[[BB10]]
; CHECK: [[BB10]]:
; CHECK-NEXT: [[I11:%.*]] = phi ptr [ [[I7]], %[[DOTLOOPEXIT49_1]] ], [ null, %[[DEAD2]] ]
-; CHECK-NEXT: [[TMP0:%.*]] = load <2 x i64>, ptr [[I11]], align 1
-; CHECK-NEXT: [[TMP1:%.*]] = shufflevector <2 x i64> [[TMP0]], <2 x i64> poison, <2 x i32> <i32 1, i32 0>
-; CHECK-NEXT: store <2 x i64> [[TMP1]], ptr [[I6]], align 1
+; CHECK-NEXT: [[I16:%.*]] = getelementptr i8, ptr [[I11]], i64 8
+; CHECK-NEXT: [[I17:%.*]] = load i64, ptr [[I16]], align 1
+; CHECK-NEXT: store i64 [[I17]], ptr [[I6]], align 1
+; CHECK-NEXT: [[I18:%.*]] = getelementptr i8, ptr [[I6]], i64 8
+; CHECK-NEXT: [[I19:%.*]] = load i64, ptr [[I11]], align 1
+; CHECK-NEXT: store i64 [[I19]], ptr [[I18]], align 1
; CHECK-NEXT: br label %[[BB20]]
; CHECK: [[BB20]]:
; CHECK-NEXT: [[I21]] = phi ptr [ [[I5]], %[[DEAD1]] ], [ [[I6]], %[[BB10]] ]
-; CHECK-NEXT: br label %[[LOOP1]]
+; CHECK-NEXT: br label %[[_LOOPEXIT49_1]]
; CHECK: [[BB22]]:
; CHECK-NEXT: [[I23:%.*]] = getelementptr i8, ptr [[I]], i64 [[ARG2]]
; CHECK-NEXT: [[I25:%.*]] = getelementptr i8, ptr [[I23]], i64 8
>From 8fac86fc395effee9df21f3ad70fe84bc7e7da72 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Thu, 25 Jul 2024 09:23:04 +0200
Subject: [PATCH 3/3] Expand comment
---
llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll b/llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll
index 3d14d286eff4f..429bf13b2b87a 100644
--- a/llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll
+++ b/llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll
@@ -3,7 +3,10 @@
target triple = "x86_64-redhat-linux-gnu"
-; Should not get vectorized.
+; The load+store sequence inside bb10 should not get vectorized. Previously,
+; we incorrectly determined that the pointers do not alias, because a cache
+; entry based indirectly on a disproven NoAlias assumption was not cleared
+; from the BatchAA cache.
define void @test(ptr %p1, i64 %arg1, i64 %arg2) {
; CHECK-LABEL: define void @test(
; CHECK-SAME: ptr [[P1:%.*]], i64 [[ARG1:%.*]], i64 [[ARG2:%.*]]) {
More information about the llvm-commits
mailing list