Skip to content

Commit ce2518f

Browse files
jinhuang1102Jin Huang
jinhuang1102
and
Jin Huang
authored
[AA] Improve precision for monotonic atomic load/store operations (#158169)
Refines `getModRefInfo` for `Monotonic` and `Unordered` atomic operations to perform a full alias evaluation instead of conservatively returning `ModRef`. This allows AA to return more precise results (e.g., `NoModRef`), unblocking optimizations in passes like DSE/PRE when the pointers are proven not to alias. LIT testing: 1. Fixed the `XFAIL` test `@test9` in `DSE/atomic-todo.ll`, where the improved AA now allows DSE to correctly remove a dead store. 2. Added test `@test9a` to the same file to ensure DSE is still blocked when pointers may alias. 3. I am also reviewing tests in GVN/PRE to see where this change may be beneficial. While all existing tests pass, I will update this PR with any new GVN test cases in a follow-up commit. --------- Co-authored-by: Jin Huang <[email protected]>
1 parent dedcf88 commit ce2518f

File tree

3 files changed

+83
-52
lines changed

3 files changed

+83
-52
lines changed

‎llvm/lib/Analysis/AliasAnalysis.cpp‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ ModRefInfo AAResults::getModRefInfo(const LoadInst *L,
433433
const MemoryLocation &Loc,
434434
AAQueryInfo &AAQI) {
435435
// Be conservative in the face of atomic.
436-
if (isStrongerThan(L->getOrdering(), AtomicOrdering::Unordered))
436+
if (isStrongerThanMonotonic(L->getOrdering()))
437437
return ModRefInfo::ModRef;
438438

439439
// If the load address doesn't alias the given address, it doesn't read
@@ -443,6 +443,13 @@ ModRefInfo AAResults::getModRefInfo(const LoadInst *L,
443443
if (AR == AliasResult::NoAlias)
444444
return ModRefInfo::NoModRef;
445445
}
446+
447+
assert(!isStrongerThanMonotonic(L->getOrdering()) &&
448+
"Stronger atomic orderings should have been handled above!");
449+
450+
if (isStrongerThanUnordered(L->getOrdering()))
451+
return ModRefInfo::ModRef;
452+
446453
// Otherwise, a load just reads.
447454
return ModRefInfo::Ref;
448455
}
@@ -451,7 +458,7 @@ ModRefInfo AAResults::getModRefInfo(const StoreInst *S,
451458
const MemoryLocation &Loc,
452459
AAQueryInfo &AAQI) {
453460
// Be conservative in the face of atomic.
454-
if (isStrongerThan(S->getOrdering(), AtomicOrdering::Unordered))
461+
if (isStrongerThanMonotonic(S->getOrdering()))
455462
return ModRefInfo::ModRef;
456463

457464
if (Loc.Ptr) {
@@ -469,7 +476,13 @@ ModRefInfo AAResults::getModRefInfo(const StoreInst *S,
469476
return ModRefInfo::NoModRef;
470477
}
471478

472-
// Otherwise, a store just writes.
479+
assert(!isStrongerThanMonotonic(S->getOrdering()) &&
480+
"Stronger atomic orderings should have been handled above!");
481+
482+
if (isStrongerThanUnordered(S->getOrdering()))
483+
return ModRefInfo::ModRef;
484+
485+
// A store just writes.
473486
return ModRefInfo::Mod;
474487
}
475488

‎llvm/test/Transforms/DeadStoreElimination/atomic-todo.ll‎

Lines changed: 0 additions & 23 deletions
This file was deleted.

‎llvm/test/Transforms/DeadStoreElimination/atomic.ll‎

Lines changed: 67 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,21 @@ define void @test4() {
3737
ret void
3838
}
3939

40-
; DSE unordered store overwriting non-atomic store (allowed)
40+
; DSE doesn't remove monotonic store.
4141
define void @test5() {
4242
; CHECK-LABEL: @test5(
43+
; CHECK-NEXT: store atomic i32 2, ptr @x monotonic, align 4
44+
; CHECK-NEXT: store i32 1, ptr @x, align 4
45+
; CHECK-NEXT: ret void
46+
;
47+
store atomic i32 2, ptr @x monotonic, align 4
48+
store i32 1, ptr @x
49+
ret void
50+
}
51+
52+
; DSE unordered store overwriting non-atomic store (allowed)
53+
define void @test6() {
54+
; CHECK-LABEL: @test6(
4355
; CHECK-NEXT: store atomic i32 1, ptr @x unordered, align 4
4456
; CHECK-NEXT: ret void
4557
;
@@ -49,8 +61,8 @@ define void @test5() {
4961
}
5062

5163
; DSE no-op unordered atomic store (allowed)
52-
define void @test6() {
53-
; CHECK-LABEL: @test6(
64+
define void @test7() {
65+
; CHECK-LABEL: @test7(
5466
; CHECK-NEXT: ret void
5567
;
5668
%x = load atomic i32, ptr @x unordered, align 4
@@ -60,8 +72,8 @@ define void @test6() {
6072

6173
; DSE seq_cst store (be conservative; DSE doesn't have infrastructure
6274
; to reason about atomic operations).
63-
define void @test7() {
64-
; CHECK-LABEL: @test7(
75+
define void @test8() {
76+
; CHECK-LABEL: @test8(
6577
; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4
6678
; CHECK-NEXT: store atomic i32 0, ptr [[A]] seq_cst, align 4
6779
; CHECK-NEXT: ret void
@@ -73,8 +85,8 @@ define void @test7() {
7385

7486
; DSE and seq_cst load (be conservative; DSE doesn't have infrastructure
7587
; to reason about atomic operations).
76-
define i32 @test8() {
77-
; CHECK-LABEL: @test8(
88+
define i32 @test9() {
89+
; CHECK-LABEL: @test9(
7890
; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4
7991
; CHECK-NEXT: call void @randomop(ptr [[A]])
8092
; CHECK-NEXT: store i32 0, ptr [[A]], align 4
@@ -88,20 +100,49 @@ define i32 @test8() {
88100
ret i32 %x
89101
}
90102

103+
; DSE across monotonic load (allowed if the monotonic load's address is NoAlias)
104+
define i32 @test10() {
105+
; CHECK-LABEL: @test10(
106+
; CHECK-NEXT: [[X:%.*]] = load atomic i32, ptr @y monotonic, align 4
107+
; CHECK-NEXT: store i32 1, ptr @x, align 4
108+
; CHECK-NEXT: ret i32 [[X]]
109+
;
110+
store i32 0, ptr @x
111+
%x = load atomic i32, ptr @y monotonic, align 4
112+
store i32 1, ptr @x
113+
ret i32 %x
114+
}
115+
116+
; DSE across monotonic load (blocked if the atomic load's address isn't NoAlias)
117+
define i32 @test11(ptr %ptr) {
118+
; CHECK-LABEL: @test11(
119+
; CHECK-NEXT: store i32 0, ptr @x, align 4
120+
; CHECK-NEXT: [[X:%.*]] = load atomic i32, ptr [[PTR:%.*]] monotonic, align 4
121+
; CHECK-NEXT: store i32 1, ptr @x, align 4
122+
; CHECK-NEXT: ret i32 [[X]]
123+
;
124+
store i32 0, ptr @x
125+
%x = load atomic i32, ptr %ptr monotonic, align 4
126+
store i32 1, ptr @x
127+
ret i32 %x
128+
}
129+
91130
; DSE across monotonic store (allowed as long as the eliminated store isUnordered)
92-
define void @test10() {
93-
; CHECK-LABEL: test10
94-
; CHECK-NOT: store i32 0
95-
; CHECK: store i32 1
131+
define void @test12() {
132+
; CHECK-LABEL: @test12(
133+
; CHECK-NEXT: store atomic i32 42, ptr @y monotonic, align 4
134+
; CHECK-NEXT: store i32 1, ptr @x, align 4
135+
; CHECK-NEXT: ret void
136+
;
96137
store i32 0, ptr @x
97138
store atomic i32 42, ptr @y monotonic, align 4
98139
store i32 1, ptr @x
99140
ret void
100141
}
101142

102143
; DSE across monotonic load (forbidden since the eliminated store is atomic)
103-
define i32 @test11() {
104-
; CHECK-LABEL: @test11(
144+
define i32 @test13() {
145+
; CHECK-LABEL: @test13(
105146
; CHECK-NEXT: store atomic i32 0, ptr @x monotonic, align 4
106147
; CHECK-NEXT: [[X:%.*]] = load atomic i32, ptr @y monotonic, align 4
107148
; CHECK-NEXT: store atomic i32 1, ptr @x monotonic, align 4
@@ -114,8 +155,8 @@ define i32 @test11() {
114155
}
115156

116157
; DSE across monotonic store (forbidden since the eliminated store is atomic)
117-
define void @test12() {
118-
; CHECK-LABEL: @test12(
158+
define void @test14() {
159+
; CHECK-LABEL: @test14(
119160
; CHECK-NEXT: store atomic i32 0, ptr @x monotonic, align 4
120161
; CHECK-NEXT: store atomic i32 42, ptr @y monotonic, align 4
121162
; CHECK-NEXT: store atomic i32 1, ptr @x monotonic, align 4
@@ -150,7 +191,7 @@ define i32 @test15() {
150191
define i64 @test_atomicrmw_0() {
151192
; CHECK-LABEL: @test_atomicrmw_0(
152193
; CHECK-NEXT: store i64 1, ptr @z, align 8
153-
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @z, i64 -1 monotonic
194+
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @z, i64 -1 monotonic, align 8
154195
; CHECK-NEXT: ret i64 [[RES]]
155196
;
156197
store i64 1, ptr @z
@@ -162,7 +203,7 @@ define i64 @test_atomicrmw_0() {
162203
define i64 @test_atomicrmw_1() {
163204
; CHECK-LABEL: @test_atomicrmw_1(
164205
; CHECK-NEXT: store i64 1, ptr @z, align 8
165-
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @z, i64 -1 acq_rel
206+
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @z, i64 -1 acq_rel, align 8
166207
; CHECK-NEXT: ret i64 [[RES]]
167208
;
168209
store i64 1, ptr @z
@@ -173,7 +214,7 @@ define i64 @test_atomicrmw_1() {
173214
; Monotonic atomicrmw should not block eliminating no-aliasing stores.
174215
define i64 @test_atomicrmw_2() {
175216
; CHECK-LABEL: @test_atomicrmw_2(
176-
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @a, i64 -1 monotonic
217+
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @a, i64 -1 monotonic, align 8
177218
; CHECK-NEXT: store i64 2, ptr @z, align 8
178219
; CHECK-NEXT: ret i64 [[RES]]
179220
;
@@ -187,7 +228,7 @@ define i64 @test_atomicrmw_2() {
187228
define i64 @test_atomicrmw_3() {
188229
; CHECK-LABEL: @test_atomicrmw_3(
189230
; CHECK-NEXT: store i64 1, ptr @z, align 8
190-
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @a, i64 -1 release
231+
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @a, i64 -1 release, align 8
191232
; CHECK-NEXT: store i64 2, ptr @z, align 8
192233
; CHECK-NEXT: ret i64 [[RES]]
193234
;
@@ -201,7 +242,7 @@ define i64 @test_atomicrmw_3() {
201242
define i64 @test_atomicrmw_4(ptr %ptr) {
202243
; CHECK-LABEL: @test_atomicrmw_4(
203244
; CHECK-NEXT: store i64 1, ptr @z, align 8
204-
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr [[PTR:%.*]], i64 -1 monotonic
245+
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr [[PTR:%.*]], i64 -1 monotonic, align 8
205246
; CHECK-NEXT: store i64 2, ptr @z, align 8
206247
; CHECK-NEXT: ret i64 [[RES]]
207248
;
@@ -215,7 +256,7 @@ define i64 @test_atomicrmw_4(ptr %ptr) {
215256
define i64 @test_atomicrmw_5() {
216257
; CHECK-LABEL: @test_atomicrmw_5(
217258
; CHECK-NEXT: store i64 1, ptr @z, align 8
218-
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @z, i64 -1 monotonic
259+
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @z, i64 -1 monotonic, align 8
219260
; CHECK-NEXT: store i64 2, ptr @z, align 8
220261
; CHECK-NEXT: ret i64 [[RES]]
221262
;
@@ -229,7 +270,7 @@ define i64 @test_atomicrmw_5() {
229270
define { i32, i1} @test_cmpxchg_1() {
230271
; CHECK-LABEL: @test_cmpxchg_1(
231272
; CHECK-NEXT: store i32 1, ptr @x, align 4
232-
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @x, i32 10, i32 20 seq_cst monotonic
273+
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @x, i32 10, i32 20 seq_cst monotonic, align 4
233274
; CHECK-NEXT: store i32 2, ptr @x, align 4
234275
; CHECK-NEXT: ret { i32, i1 } [[RET]]
235276
;
@@ -242,7 +283,7 @@ define { i32, i1} @test_cmpxchg_1() {
242283
; Monotonic cmpxchg should not block DSE for non-aliasing stores.
243284
define { i32, i1} @test_cmpxchg_2() {
244285
; CHECK-LABEL: @test_cmpxchg_2(
245-
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @y, i32 10, i32 20 monotonic monotonic
286+
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @y, i32 10, i32 20 monotonic monotonic, align 4
246287
; CHECK-NEXT: store i32 2, ptr @x, align 4
247288
; CHECK-NEXT: ret { i32, i1 } [[RET]]
248289
;
@@ -256,7 +297,7 @@ define { i32, i1} @test_cmpxchg_2() {
256297
define { i32, i1} @test_cmpxchg_3() {
257298
; CHECK-LABEL: @test_cmpxchg_3(
258299
; CHECK-NEXT: store i32 1, ptr @x, align 4
259-
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @y, i32 10, i32 20 seq_cst seq_cst
300+
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @y, i32 10, i32 20 seq_cst seq_cst, align 4
260301
; CHECK-NEXT: store i32 2, ptr @x, align 4
261302
; CHECK-NEXT: ret { i32, i1 } [[RET]]
262303
;
@@ -270,7 +311,7 @@ define { i32, i1} @test_cmpxchg_3() {
270311
define { i32, i1} @test_cmpxchg_4(ptr %ptr) {
271312
; CHECK-LABEL: @test_cmpxchg_4(
272313
; CHECK-NEXT: store i32 1, ptr @x, align 4
273-
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr [[PTR:%.*]], i32 10, i32 20 monotonic monotonic
314+
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr [[PTR:%.*]], i32 10, i32 20 monotonic monotonic, align 4
274315
; CHECK-NEXT: store i32 2, ptr @x, align 4
275316
; CHECK-NEXT: ret { i32, i1 } [[RET]]
276317
;
@@ -284,7 +325,7 @@ define { i32, i1} @test_cmpxchg_4(ptr %ptr) {
284325
define { i32, i1} @test_cmpxchg_5(ptr %ptr) {
285326
; CHECK-LABEL: @test_cmpxchg_5(
286327
; CHECK-NEXT: store i32 1, ptr @x, align 4
287-
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @x, i32 10, i32 20 monotonic monotonic
328+
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @x, i32 10, i32 20 monotonic monotonic, align 4
288329
; CHECK-NEXT: store i32 2, ptr @x, align 4
289330
; CHECK-NEXT: ret { i32, i1 } [[RET]]
290331
;

0 commit comments

Comments
 (0)