Skip to content

Commit 61b0a2f

Browse files
nikictru
nikic
authored and
tru
committed
[BasicAA] Fix handling of indirect assumption based results (#100130)
If a result is potentially based on a not yet proven assumption, BasicAA will remember it inside AssumptionBasedResults and remove the cache entry if an assumption higher up is later disproved. 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 disproved. At the end of the root query, convert remaining AssumptionBased results into definitive results. Fixes #98978. (cherry picked from commit 9107338)
1 parent 50b9db3 commit 61b0a2f

File tree

3 files changed

+144
-7
lines changed

3 files changed

+144
-7
lines changed

‎llvm/include/llvm/Analysis/AliasAnalysis.h‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -244,12 +244,23 @@ class AAQueryInfo {
244244
public:
245245
using LocPair = std::pair<AACacheLoc, AACacheLoc>;
246246
struct CacheEntry {
247+
/// Cache entry is neither an assumption nor does it use a (non-definitive)
248+
/// assumption.
249+
static constexpr int Definitive = -2;
250+
/// Cache entry is not an assumption itself, but may be using an assumption
251+
/// from higher up the stack.
252+
static constexpr int AssumptionBased = -1;
253+
247254
AliasResult Result;
248-
/// Number of times a NoAlias assumption has been used.
249-
/// 0 for assumptions that have not been used, -1 for definitive results.
255+
/// Number of times a NoAlias assumption has been used, 0 for assumptions
256+
/// that have not been used. Can also take one of the Definitive or
257+
/// AssumptionBased values documented above.
250258
int NumAssumptionUses;
259+
251260
/// Whether this is a definitive (non-assumption) result.
252-
bool isDefinitive() const { return NumAssumptionUses < 0; }
261+
bool isDefinitive() const { return NumAssumptionUses == Definitive; }
262+
/// Whether this is an assumption that has not been proven yet.
263+
bool isAssumption() const { return NumAssumptionUses >= 0; }
253264
};
254265

255266
// Alias analysis result aggregration using which this query is performed.

‎llvm/lib/Analysis/BasicAliasAnalysis.cpp‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1692,9 +1692,12 @@ AliasResult BasicAAResult::aliasCheck(const Value *V1, LocationSize V1Size,
16921692
if (!Pair.second) {
16931693
auto &Entry = Pair.first->second;
16941694
if (!Entry.isDefinitive()) {
1695-
// Remember that we used an assumption.
1696-
++Entry.NumAssumptionUses;
1695+
// Remember that we used an assumption. This may either be a direct use
1696+
// of an assumption, or a use of an entry that may itself be based on an
1697+
// assumption.
16971698
++AAQI.NumAssumptionUses;
1699+
if (Entry.isAssumption())
1700+
++Entry.NumAssumptionUses;
16981701
}
16991702
// Cache contains sorted {V1,V2} pairs but we should return original order.
17001703
auto Result = Entry.Result;
@@ -1722,7 +1725,6 @@ AliasResult BasicAAResult::aliasCheck(const Value *V1, LocationSize V1Size,
17221725
Entry.Result = Result;
17231726
// Cache contains sorted {V1,V2} pairs.
17241727
Entry.Result.swap(Swapped);
1725-
Entry.NumAssumptionUses = -1;
17261728

17271729
// If the assumption has been disproven, remove any results that may have
17281730
// 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,
17341736
// The result may still be based on assumptions higher up in the chain.
17351737
// Remember it, so it can be purged from the cache later.
17361738
if (OrigNumAssumptionUses != AAQI.NumAssumptionUses &&
1737-
Result != AliasResult::MayAlias)
1739+
Result != AliasResult::MayAlias) {
17381740
AAQI.AssumptionBasedResults.push_back(Locs);
1741+
Entry.NumAssumptionUses = AAQueryInfo::CacheEntry::AssumptionBased;
1742+
} else {
1743+
Entry.NumAssumptionUses = AAQueryInfo::CacheEntry::Definitive;
1744+
}
1745+
1746+
// Depth is incremented before this function is called, so Depth==1 indicates
1747+
// a root query.
1748+
if (AAQI.Depth == 1) {
1749+
// Any remaining assumption based results must be based on proven
1750+
// assumptions, so convert them to definitive results.
1751+
for (const auto &Loc : AAQI.AssumptionBasedResults) {
1752+
auto It = AAQI.AliasCache.find(Loc);
1753+
if (It != AAQI.AliasCache.end())
1754+
It->second.NumAssumptionUses = AAQueryInfo::CacheEntry::Definitive;
1755+
}
1756+
AAQI.AssumptionBasedResults.clear();
1757+
AAQI.NumAssumptionUses = 0;
1758+
}
17391759
return Result;
17401760
}
17411761

‎llvm/test/Transforms/SLPVectorizer/X86/pr98978.ll‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
2+
; RUN: opt -S -passes=slp-vectorizer < %s | FileCheck %s
3+
4+
target triple = "x86_64-redhat-linux-gnu"
5+
6+
; The load+store sequence inside bb10 should not get vectorized. Previously,
7+
; we incorrectly determined that the pointers do not alias, because a cache
8+
; entry based indirectly on a disproven NoAlias assumption was not cleared
9+
; from the BatchAA cache.
10+
define void @test(ptr %p1, i64 %arg1, i64 %arg2) {
11+
; CHECK-LABEL: define void @test(
12+
; CHECK-SAME: ptr [[P1:%.*]], i64 [[ARG1:%.*]], i64 [[ARG2:%.*]]) {
13+
; CHECK-NEXT: [[_PREHEADER48_PREHEADER_1:.*]]:
14+
; CHECK-NEXT: br label %[[_LOOPEXIT49_1:.*]]
15+
; CHECK: [[_LOOPEXIT49_1]]:
16+
; CHECK-NEXT: [[I:%.*]] = phi ptr [ [[I21:%.*]], %[[BB20:.*]] ], [ [[P1]], %[[_PREHEADER48_PREHEADER_1]] ]
17+
; CHECK-NEXT: br i1 false, label %[[BB22:.*]], label %[[DOTPREHEADER48_PREHEADER_1:.*]]
18+
; CHECK: [[DEAD:.*]]:
19+
; CHECK-NEXT: br label %[[DOTPREHEADER48_PREHEADER_1]]
20+
; CHECK: [[_PREHEADER48_PREHEADER_2:.*:]]
21+
; CHECK-NEXT: [[I5:%.*]] = phi ptr [ [[I]], %[[DEAD]] ], [ [[I]], %[[_LOOPEXIT49_1]] ]
22+
; CHECK-NEXT: br label %[[DOTLOOPEXIT49_1:.*]]
23+
; CHECK: [[DEAD1:.*]]:
24+
; CHECK-NEXT: br i1 false, label %[[DOTLOOPEXIT49_1]], label %[[BB20]]
25+
; CHECK: [[_LOOPEXIT49_2:.*:]]
26+
; CHECK-NEXT: [[I6:%.*]] = phi ptr [ [[I5]], %[[DEAD1]] ], [ [[I5]], %[[DOTPREHEADER48_PREHEADER_1]] ]
27+
; CHECK-NEXT: [[I7:%.*]] = getelementptr i8, ptr [[I6]], i64 [[ARG1]]
28+
; CHECK-NEXT: br label %[[BB10:.*]]
29+
; CHECK: [[DEAD2:.*]]:
30+
; CHECK-NEXT: br label %[[BB10]]
31+
; CHECK: [[BB10]]:
32+
; CHECK-NEXT: [[I11:%.*]] = phi ptr [ [[I7]], %[[DOTLOOPEXIT49_1]] ], [ null, %[[DEAD2]] ]
33+
; CHECK-NEXT: [[I16:%.*]] = getelementptr i8, ptr [[I11]], i64 8
34+
; CHECK-NEXT: [[I17:%.*]] = load i64, ptr [[I16]], align 1
35+
; CHECK-NEXT: store i64 [[I17]], ptr [[I6]], align 1
36+
; CHECK-NEXT: [[I18:%.*]] = getelementptr i8, ptr [[I6]], i64 8
37+
; CHECK-NEXT: [[I19:%.*]] = load i64, ptr [[I11]], align 1
38+
; CHECK-NEXT: store i64 [[I19]], ptr [[I18]], align 1
39+
; CHECK-NEXT: br label %[[BB20]]
40+
; CHECK: [[BB20]]:
41+
; CHECK-NEXT: [[I21]] = phi ptr [ [[I5]], %[[DEAD1]] ], [ [[I6]], %[[BB10]] ]
42+
; CHECK-NEXT: br label %[[_LOOPEXIT49_1]]
43+
; CHECK: [[BB22]]:
44+
; CHECK-NEXT: [[I23:%.*]] = getelementptr i8, ptr [[I]], i64 [[ARG2]]
45+
; CHECK-NEXT: [[I25:%.*]] = getelementptr i8, ptr [[I23]], i64 8
46+
; CHECK-NEXT: br label %[[BB26:.*]]
47+
; CHECK: [[BB26]]:
48+
; CHECK-NEXT: [[I27:%.*]] = phi ptr [ null, %[[BB26]] ], [ [[I25]], %[[BB22]] ]
49+
; CHECK-NEXT: store i64 0, ptr [[I27]], align 1
50+
; CHECK-NEXT: [[I28:%.*]] = getelementptr i8, ptr [[I27]], i64 8
51+
; CHECK-NEXT: [[I29:%.*]] = load i64, ptr [[I23]], align 1
52+
; CHECK-NEXT: store i64 0, ptr [[I28]], align 1
53+
; CHECK-NEXT: br label %[[BB26]]
54+
;
55+
entry:
56+
br label %loop1
57+
58+
loop1: ; preds = %bb20, %entry
59+
%i = phi ptr [ %i21, %bb20 ], [ %p1, %entry ]
60+
br i1 false, label %bb22, label %.preheader48.preheader.1
61+
62+
dead: ; No predecessors!
63+
br label %.preheader48.preheader.1
64+
65+
.preheader48.preheader.1: ; preds = %dead, %loop1
66+
%i5 = phi ptr [ %i, %dead ], [ %i, %loop1 ]
67+
br label %.loopexit49.1
68+
69+
dead1: ; No predecessors!
70+
br i1 false, label %.loopexit49.1, label %bb20
71+
72+
.loopexit49.1: ; preds = %dead1, %.preheader48.preheader.1
73+
%i6 = phi ptr [ %i5, %dead1 ], [ %i5, %.preheader48.preheader.1 ]
74+
%i7 = getelementptr i8, ptr %i6, i64 %arg1
75+
br label %bb10
76+
77+
dead2: ; No predecessors!
78+
br label %bb10
79+
80+
bb10: ; preds = %dead2, %.loopexit49.1
81+
%i11 = phi ptr [ %i7, %.loopexit49.1 ], [ null, %dead2 ]
82+
%i16 = getelementptr i8, ptr %i11, i64 8
83+
%i17 = load i64, ptr %i16, align 1
84+
store i64 %i17, ptr %i6, align 1
85+
%i18 = getelementptr i8, ptr %i6, i64 8
86+
%i19 = load i64, ptr %i11, align 1
87+
store i64 %i19, ptr %i18, align 1
88+
br label %bb20
89+
90+
bb20: ; preds = %bb10, %dead1
91+
%i21 = phi ptr [ %i5, %dead1 ], [ %i6, %bb10 ]
92+
br label %loop1
93+
94+
bb22: ; preds = %loop1
95+
%i23 = getelementptr i8, ptr %i, i64 %arg2
96+
%i25 = getelementptr i8, ptr %i23, i64 8
97+
br label %bb26
98+
99+
bb26: ; preds = %bb26, %bb22
100+
%i27 = phi ptr [ null, %bb26 ], [ %i25, %bb22 ]
101+
store i64 0, ptr %i27, align 1
102+
%i28 = getelementptr i8, ptr %i27, i64 8
103+
%i29 = load i64, ptr %i23, align 1
104+
store i64 0, ptr %i28, align 1
105+
br label %bb26
106+
}

0 commit comments

Comments
 (0)