Skip to content

Commit fed817a

Browse files
authored
[DSE] Consider the aliasing through global variable while checking clobber (#120044)
While update the read clobber check for the "initializes" attr, we checked the aliasing among arguments, but didn't consider the aliasing through global variable. It causes problems in this example: ``` int g_var = 123; void update(int* ptr) { *ptr = g_var; void foo() { g_var = 0; bar(&g_var); } ``` We mistakenly removed `g_var = 0;` as a dead store. Fix the issue by requiring the CallBase only access argmem or inaccessiblemem.
1 parent c7fddf5 commit fed817a

File tree

2 files changed

+61
-7
lines changed

2 files changed

+61
-7
lines changed

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2262,6 +2262,14 @@ struct DSEState {
22622262
bool eliminateDeadDefs(const MemoryDefWrapper &KillingDefWrapper);
22632263
};
22642264

2265+
// Return true if "Arg" is function local and isn't captured before "CB".
2266+
bool isFuncLocalAndNotCaptured(Value *Arg, const CallBase *CB,
2267+
EarliestEscapeAnalysis &EA) {
2268+
const Value *UnderlyingObj = getUnderlyingObject(Arg);
2269+
return isIdentifiedFunctionLocal(UnderlyingObj) &&
2270+
EA.isNotCapturedBefore(UnderlyingObj, CB, /*OrAt*/ true);
2271+
}
2272+
22652273
SmallVector<MemoryLocation, 1>
22662274
DSEState::getInitializesArgMemLoc(const Instruction *I) {
22672275
const CallBase *CB = dyn_cast<CallBase>(I);
@@ -2277,6 +2285,13 @@ DSEState::getInitializesArgMemLoc(const Instruction *I) {
22772285
Inits = InitializesAttr.getValueAsConstantRangeList();
22782286

22792287
Value *CurArg = CB->getArgOperand(Idx);
2288+
// Check whether "CurArg" could alias with global variables. We require
2289+
// either it's function local and isn't captured before or the "CB" only
2290+
// accesses arg or inaccessible mem.
2291+
if (!Inits.empty() && !CB->onlyAccessesInaccessibleMemOrArgMem() &&
2292+
!isFuncLocalAndNotCaptured(CurArg, CB, EA))
2293+
Inits = ConstantRangeList();
2294+
22802295
// We don't perform incorrect DSE on unwind edges in the current function,
22812296
// and use the "initializes" attribute to kill dead stores if:
22822297
// - The call does not throw exceptions, "CB->doesNotThrow()".

llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,25 @@ define i16 @p1_write_then_read_caller() {
3939
ret i16 %l
4040
}
4141

42+
declare void @fn_capture(ptr)
43+
define i16 @p1_write_then_read_caller_escape() {
44+
; CHECK-LABEL: @p1_write_then_read_caller_escape(
45+
; CHECK-NEXT: [[PTR:%.*]] = alloca i16, align 2
46+
; CHECK-NEXT: store i16 0, ptr [[PTR]], align 2
47+
; CHECK-NEXT: call void @fn_capture(ptr [[PTR]])
48+
; CHECK-NEXT: call void @p1_write_then_read(ptr [[PTR]])
49+
; CHECK-NEXT: [[L:%.*]] = load i16, ptr [[PTR]], align 2
50+
; CHECK-NEXT: ret i16 [[L]]
51+
;
52+
%ptr = alloca i16
53+
store i16 0, ptr %ptr
54+
call void @fn_capture(ptr %ptr)
55+
call void @p1_write_then_read(ptr %ptr)
56+
%l = load i16, ptr %ptr
57+
ret i16 %l
58+
}
59+
60+
4261
; Function Attrs: mustprogress nounwind uwtable
4362
define i16 @p1_write_then_read_caller_with_clobber() {
4463
; CHECK-LABEL: @p1_write_then_read_caller_with_clobber(
@@ -221,15 +240,17 @@ declare void @large_p2(ptr nocapture noundef initializes((0, 200)), ptr nocaptur
221240
; Function Attrs: mustprogress nounwind uwtable
222241
define i16 @large_p1_caller() {
223242
; CHECK-LABEL: @large_p1_caller(
224-
; CHECK-NEXT: [[PTR:%.*]] = alloca [200 x i8], align 1
225-
; CHECK-NEXT: call void @large_p1(ptr [[PTR]])
226-
; CHECK-NEXT: [[L:%.*]] = load i16, ptr [[PTR]], align 2
243+
; CHECK-NEXT: [[PTR:%.*]] = alloca [300 x i8], align 1
244+
; CHECK-NEXT: [[TMP:%.*]] = getelementptr i8, ptr [[PTR]], i64 100
245+
; CHECK-NEXT: call void @large_p1(ptr [[TMP]])
246+
; CHECK-NEXT: [[L:%.*]] = load i16, ptr [[TMP]], align 2
227247
; CHECK-NEXT: ret i16 [[L]]
228248
;
229-
%ptr = alloca [200 x i8]
230-
call void @llvm.memset.p0.i64(ptr %ptr, i8 42, i64 100, i1 false)
231-
call void @large_p1(ptr %ptr)
232-
%l = load i16, ptr %ptr
249+
%ptr = alloca [300 x i8]
250+
%tmp = getelementptr i8, ptr %ptr, i64 100
251+
call void @llvm.memset.p0.i64(ptr %tmp, i8 42, i64 100, i1 false)
252+
call void @large_p1(ptr %tmp)
253+
%l = load i16, ptr %tmp
233254
ret i16 %l
234255
}
235256

@@ -299,3 +320,21 @@ define i16 @large_p2_may_or_partial_alias_caller2(ptr %base1, ptr %base2) {
299320
ret i16 %l
300321
}
301322

323+
@g = global i16 123, align 2
324+
325+
declare void @read_global(ptr nocapture noundef initializes((0, 2))) nounwind
326+
memory(read, argmem: write, inaccessiblemem: none) nounwind
327+
328+
define i16 @global_var_alias() {
329+
; CHECK-LABEL: @global_var_alias(
330+
; CHECK-NEXT: store i16 0, ptr @g, align 4
331+
; CHECK-NEXT: call void @read_global(ptr @g)
332+
; CHECK-NEXT: [[L:%.*]] = load i16, ptr @g, align 2
333+
; CHECK-NEXT: ret i16 [[L]]
334+
;
335+
store i16 0, ptr @g, align 4
336+
call void @read_global(ptr @g)
337+
%l = load i16, ptr @g
338+
ret i16 %l
339+
}
340+

0 commit comments

Comments
 (0)