Skip to content

Commit c8941df

Browse files
jinhuang1102jinhuang1102
authored
Revert "[AA] Improve precision for monotonic atomic load/store operations" (#173135)
Reverts #158169 The improved AA precision for atomic store operations causes the DSE pass to optimize out the object variables.
1 parent e8eb20b commit c8941df

3 files changed

Lines changed: 52 additions & 83 deletions

File tree

‎llvm/lib/Analysis/AliasAnalysis.cpp‎

Lines changed: 3 additions & 16 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 (isStrongerThanMonotonic(L->getOrdering()))
436+
if (isStrongerThan(L->getOrdering(), AtomicOrdering::Unordered))
437437
return ModRefInfo::ModRef;
438438

439439
// If the load address doesn't alias the given address, it doesn't read
@@ -443,13 +443,6 @@ 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-
453446
// Otherwise, a load just reads.
454447
return ModRefInfo::Ref;
455448
}
@@ -458,7 +451,7 @@ ModRefInfo AAResults::getModRefInfo(const StoreInst *S,
458451
const MemoryLocation &Loc,
459452
AAQueryInfo &AAQI) {
460453
// Be conservative in the face of atomic.
461-
if (isStrongerThanMonotonic(S->getOrdering()))
454+
if (isStrongerThan(S->getOrdering(), AtomicOrdering::Unordered))
462455
return ModRefInfo::ModRef;
463456

464457
if (Loc.Ptr) {
@@ -476,13 +469,7 @@ ModRefInfo AAResults::getModRefInfo(const StoreInst *S,
476469
return ModRefInfo::NoModRef;
477470
}
478471

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.
472+
// Otherwise, a store just writes.
486473
return ModRefInfo::Mod;
487474
}
488475

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
; XFAIL: *
2+
; RUN: opt -passes=dse -S < %s | FileCheck %s
3+
4+
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
5+
target triple = "x86_64-apple-macosx10.7.0"
6+
7+
; Basic correctness tests for atomic stores.
8+
; Note that it turns out essentially every transformation DSE does is legal on
9+
; atomic ops, just some transformations are not allowed across release-acquire pairs.
10+
11+
@x = common global i32 0, align 4
12+
@y = common global i32 0, align 4
13+
14+
; DSE across monotonic load (allowed as long as the eliminated store isUnordered)
15+
define i32 @test9() {
16+
; CHECK-LABEL: test9
17+
; CHECK-NOT: store i32 0
18+
; CHECK: store i32 1
19+
store i32 0, ptr @x
20+
%x = load atomic i32, ptr @y monotonic, align 4
21+
store i32 1, ptr @x
22+
ret i32 %x
23+
}

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

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

40-
; DSE doesn't remove monotonic store.
40+
; DSE unordered store overwriting non-atomic store (allowed)
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(
5543
; CHECK-NEXT: store atomic i32 1, ptr @x unordered, align 4
5644
; CHECK-NEXT: ret void
5745
;
@@ -61,8 +49,8 @@ define void @test6() {
6149
}
6250

6351
; DSE no-op unordered atomic store (allowed)
64-
define void @test7() {
65-
; CHECK-LABEL: @test7(
52+
define void @test6() {
53+
; CHECK-LABEL: @test6(
6654
; CHECK-NEXT: ret void
6755
;
6856
%x = load atomic i32, ptr @x unordered, align 4
@@ -72,8 +60,8 @@ define void @test7() {
7260

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

8674
; DSE and seq_cst load (be conservative; DSE doesn't have infrastructure
8775
; to reason about atomic operations).
88-
define i32 @test9() {
89-
; CHECK-LABEL: @test9(
76+
define i32 @test8() {
77+
; CHECK-LABEL: @test8(
9078
; CHECK-NEXT: [[A:%.*]] = alloca i32, align 4
9179
; CHECK-NEXT: call void @randomop(ptr [[A]])
9280
; CHECK-NEXT: store i32 0, ptr [[A]], align 4
@@ -100,49 +88,20 @@ define i32 @test9() {
10088
ret i32 %x
10189
}
10290

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-
13091
; DSE across monotonic store (allowed as long as the eliminated store isUnordered)
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-
;
92+
define void @test10() {
93+
; CHECK-LABEL: test10
94+
; CHECK-NOT: store i32 0
95+
; CHECK: store i32 1
13796
store i32 0, ptr @x
13897
store atomic i32 42, ptr @y monotonic, align 4
13998
store i32 1, ptr @x
14099
ret void
141100
}
142101

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

157116
; DSE across monotonic store (forbidden since the eliminated store is atomic)
158-
define void @test14() {
159-
; CHECK-LABEL: @test14(
117+
define void @test12() {
118+
; CHECK-LABEL: @test12(
160119
; CHECK-NEXT: store atomic i32 0, ptr @x monotonic, align 4
161120
; CHECK-NEXT: store atomic i32 42, ptr @y monotonic, align 4
162121
; CHECK-NEXT: store atomic i32 1, ptr @x monotonic, align 4
@@ -191,7 +150,7 @@ define i32 @test15() {
191150
define i64 @test_atomicrmw_0() {
192151
; CHECK-LABEL: @test_atomicrmw_0(
193152
; CHECK-NEXT: store i64 1, ptr @z, align 8
194-
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @z, i64 -1 monotonic, align 8
153+
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @z, i64 -1 monotonic
195154
; CHECK-NEXT: ret i64 [[RES]]
196155
;
197156
store i64 1, ptr @z
@@ -203,7 +162,7 @@ define i64 @test_atomicrmw_0() {
203162
define i64 @test_atomicrmw_1() {
204163
; CHECK-LABEL: @test_atomicrmw_1(
205164
; CHECK-NEXT: store i64 1, ptr @z, align 8
206-
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @z, i64 -1 acq_rel, align 8
165+
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @z, i64 -1 acq_rel
207166
; CHECK-NEXT: ret i64 [[RES]]
208167
;
209168
store i64 1, ptr @z
@@ -214,7 +173,7 @@ define i64 @test_atomicrmw_1() {
214173
; Monotonic atomicrmw should not block eliminating no-aliasing stores.
215174
define i64 @test_atomicrmw_2() {
216175
; CHECK-LABEL: @test_atomicrmw_2(
217-
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @a, i64 -1 monotonic, align 8
176+
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @a, i64 -1 monotonic
218177
; CHECK-NEXT: store i64 2, ptr @z, align 8
219178
; CHECK-NEXT: ret i64 [[RES]]
220179
;
@@ -228,7 +187,7 @@ define i64 @test_atomicrmw_2() {
228187
define i64 @test_atomicrmw_3() {
229188
; CHECK-LABEL: @test_atomicrmw_3(
230189
; CHECK-NEXT: store i64 1, ptr @z, align 8
231-
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @a, i64 -1 release, align 8
190+
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @a, i64 -1 release
232191
; CHECK-NEXT: store i64 2, ptr @z, align 8
233192
; CHECK-NEXT: ret i64 [[RES]]
234193
;
@@ -242,7 +201,7 @@ define i64 @test_atomicrmw_3() {
242201
define i64 @test_atomicrmw_4(ptr %ptr) {
243202
; CHECK-LABEL: @test_atomicrmw_4(
244203
; CHECK-NEXT: store i64 1, ptr @z, align 8
245-
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr [[PTR:%.*]], i64 -1 monotonic, align 8
204+
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr [[PTR:%.*]], i64 -1 monotonic
246205
; CHECK-NEXT: store i64 2, ptr @z, align 8
247206
; CHECK-NEXT: ret i64 [[RES]]
248207
;
@@ -256,7 +215,7 @@ define i64 @test_atomicrmw_4(ptr %ptr) {
256215
define i64 @test_atomicrmw_5() {
257216
; CHECK-LABEL: @test_atomicrmw_5(
258217
; CHECK-NEXT: store i64 1, ptr @z, align 8
259-
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @z, i64 -1 monotonic, align 8
218+
; CHECK-NEXT: [[RES:%.*]] = atomicrmw add ptr @z, i64 -1 monotonic
260219
; CHECK-NEXT: store i64 2, ptr @z, align 8
261220
; CHECK-NEXT: ret i64 [[RES]]
262221
;
@@ -270,7 +229,7 @@ define i64 @test_atomicrmw_5() {
270229
define { i32, i1} @test_cmpxchg_1() {
271230
; CHECK-LABEL: @test_cmpxchg_1(
272231
; CHECK-NEXT: store i32 1, ptr @x, align 4
273-
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @x, i32 10, i32 20 seq_cst monotonic, align 4
232+
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @x, i32 10, i32 20 seq_cst monotonic
274233
; CHECK-NEXT: store i32 2, ptr @x, align 4
275234
; CHECK-NEXT: ret { i32, i1 } [[RET]]
276235
;
@@ -283,7 +242,7 @@ define { i32, i1} @test_cmpxchg_1() {
283242
; Monotonic cmpxchg should not block DSE for non-aliasing stores.
284243
define { i32, i1} @test_cmpxchg_2() {
285244
; CHECK-LABEL: @test_cmpxchg_2(
286-
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @y, i32 10, i32 20 monotonic monotonic, align 4
245+
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @y, i32 10, i32 20 monotonic monotonic
287246
; CHECK-NEXT: store i32 2, ptr @x, align 4
288247
; CHECK-NEXT: ret { i32, i1 } [[RET]]
289248
;
@@ -297,7 +256,7 @@ define { i32, i1} @test_cmpxchg_2() {
297256
define { i32, i1} @test_cmpxchg_3() {
298257
; CHECK-LABEL: @test_cmpxchg_3(
299258
; CHECK-NEXT: store i32 1, ptr @x, align 4
300-
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @y, i32 10, i32 20 seq_cst seq_cst, align 4
259+
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @y, i32 10, i32 20 seq_cst seq_cst
301260
; CHECK-NEXT: store i32 2, ptr @x, align 4
302261
; CHECK-NEXT: ret { i32, i1 } [[RET]]
303262
;
@@ -311,7 +270,7 @@ define { i32, i1} @test_cmpxchg_3() {
311270
define { i32, i1} @test_cmpxchg_4(ptr %ptr) {
312271
; CHECK-LABEL: @test_cmpxchg_4(
313272
; CHECK-NEXT: store i32 1, ptr @x, align 4
314-
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr [[PTR:%.*]], i32 10, i32 20 monotonic monotonic, align 4
273+
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr [[PTR:%.*]], i32 10, i32 20 monotonic monotonic
315274
; CHECK-NEXT: store i32 2, ptr @x, align 4
316275
; CHECK-NEXT: ret { i32, i1 } [[RET]]
317276
;
@@ -325,7 +284,7 @@ define { i32, i1} @test_cmpxchg_4(ptr %ptr) {
325284
define { i32, i1} @test_cmpxchg_5(ptr %ptr) {
326285
; CHECK-LABEL: @test_cmpxchg_5(
327286
; CHECK-NEXT: store i32 1, ptr @x, align 4
328-
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @x, i32 10, i32 20 monotonic monotonic, align 4
287+
; CHECK-NEXT: [[RET:%.*]] = cmpxchg volatile ptr @x, i32 10, i32 20 monotonic monotonic
329288
; CHECK-NEXT: store i32 2, ptr @x, align 4
330289
; CHECK-NEXT: ret { i32, i1 } [[RET]]
331290
;

0 commit comments

Comments
 (0)