Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1665,11 +1665,16 @@ struct DSEState {
// original MD. Stop walk.
// If KillingDef is a CallInst with "initializes" attribute, the reads in
// the callee would be dominated by initializations, so it should be safe.
// Note that in `getInitializesArgMemLoc`, we only check the aliasing
// among arguments. If aliasing through global variables, we consider it
// as read clobber.
bool IsKillingDefFromInitAttr = false;
if (IsInitializesAttrMemLoc) {
if (KillingI == UseInst &&
KillingUndObj == getUnderlyingObject(MaybeDeadLoc.Ptr))
IsKillingDefFromInitAttr = true;
if (auto *CB = dyn_cast<CallBase>(UseInst))
IsKillingDefFromInitAttr =
KillingI == UseInst &&
KillingUndObj == getUnderlyingObject(MaybeDeadLoc.Ptr) &&
CB->onlyAccessesInaccessibleMemOrArgMem();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make more sense to handle this inside getInitializesArgMemLoc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this check to inside getInitializesArgMemLoc. Thanks!

}

if (isReadClobber(MaybeDeadLoc, UseInst) && !IsKillingDefFromInitAttr) {
Expand Down
34 changes: 34 additions & 0 deletions llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,22 @@
; RUN: opt < %s -passes=dse -enable-dse-initializes-attr-improvement -S | FileCheck %s

declare void @p1_write_only(ptr nocapture noundef writeonly initializes((0, 2)) dead_on_unwind)

declare void @p1_write_then_read(ptr nocapture noundef initializes((0, 2)) dead_on_unwind)
memory(argmem: readwrite, inaccessiblemem: readwrite)

declare void @p1_clobber(ptr nocapture noundef)

declare void @p2_same_range(ptr nocapture noundef initializes((0, 2)) dead_on_unwind, ptr nocapture noundef initializes((0, 2)) dead_on_unwind)
memory(argmem: readwrite, inaccessiblemem: readwrite)

declare void @p2_no_init(ptr nocapture noundef initializes((0, 2)) dead_on_unwind, ptr nocapture noundef dead_on_unwind)

declare void @p2_no_dead_on_unwind(ptr nocapture noundef initializes((0, 2)) dead_on_unwind, ptr nocapture noundef initializes((0, 2)))
memory(argmem: readwrite, inaccessiblemem: readwrite)

declare void @p2_no_dead_on_unwind_but_nounwind(ptr nocapture noundef initializes((0, 2)) dead_on_unwind, ptr nocapture noundef initializes((0, 2))) nounwind
memory(argmem: readwrite, inaccessiblemem: readwrite)

; Function Attrs: mustprogress nounwind uwtable
define i16 @p1_write_only_caller() {
Expand Down Expand Up @@ -215,8 +225,12 @@ define i16 @p2_no_dead_on_unwind_but_nounwind_alias_caller() {
}

declare void @llvm.memset.p0.i64(ptr nocapture, i8, i64, i1) nounwind

declare void @large_p1(ptr nocapture noundef initializes((0, 200))) nounwind
memory(argmem: readwrite, inaccessiblemem: readwrite)

declare void @large_p2(ptr nocapture noundef initializes((0, 200)), ptr nocapture noundef initializes((0, 100))) nounwind
memory(argmem: readwrite, inaccessiblemem: readwrite)

; Function Attrs: mustprogress nounwind uwtable
define i16 @large_p1_caller() {
Expand Down Expand Up @@ -299,3 +313,23 @@ define i16 @large_p2_may_or_partial_alias_caller2(ptr %base1, ptr %base2) {
ret i16 %l
}

@g = global i16 123, align 2

declare void @read_global(ptr nocapture noundef initializes((0, 2)))
memory(read, argmem: write, inaccessiblemem: none) nounwind

define i16 @global_var_alias() {
; CHECK-LABEL: @global_var_alias(
; CHECK-NEXT: store i32 0, ptr @g, align 4
; CHECK-NEXT: [[G_ADDR:%.*]] = getelementptr i32, ptr @g, i64 1
; CHECK-NEXT: call void @read_global(ptr [[G_ADDR]])
; CHECK-NEXT: [[L:%.*]] = load i16, ptr @g, align 2
; CHECK-NEXT: ret i16 [[L]]
;
store i32 0, ptr @g, align 4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this storing an i32 value to an i16 global? and the gep is out of bounds?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, fixed the mistake. Thanks.

%g_addr = getelementptr i32, ptr @g, i64 1
call void @read_global(ptr %g_addr)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like this case would still go out of bounds for the i16 global?
%g_addr = 1 byte from the start
then @read_global initializes 2 bytes from there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops, removed the GEP and directly read/write @g. Thanks.

%l = load i16, ptr @g
ret i16 %l
}

Loading