[llvm] Consider the aliasing through global variable while checking clobber (PR #120044)

[Haopeng Liu via llvm-commits]

https://github.com/haopliu created https://github.com/llvm/llvm-project/pull/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.


>From 69d43a5976d46f97fef80e63529f4c484944c977 Mon Sep 17 00:00:00 2001
From: Haopeng Liu <haopliu at google.com>
Date: Mon, 16 Dec 2024 06:23:41 +0000
Subject: [PATCH] Consider the aliasing through global variable while checking
 read clobber

---
 .../Scalar/DeadStoreElimination.cpp           | 11 ++++--
 .../DeadStoreElimination/inter-procedural.ll  | 34 +++++++++++++++++++
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
index 4799640089fa9a..19f6bba583941b 100644
--- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -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();
       }
 
       if (isReadClobber(MaybeDeadLoc, UseInst) && !IsKillingDefFromInitAttr) {
diff --git a/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll b/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll
index d93da9b6612b05..9c040d4d5bb380 100644
--- a/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll
@@ -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() {
@@ -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() {
@@ -299,3 +313,23 @@ define i16 @large_p2_may_or_partial_alias_caller2(ptr %base1, ptr %base2) {
   ret i16 %l
 }
 
+ at 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
+  %g_addr = getelementptr i32, ptr @g, i64 1
+  call void @read_global(ptr %g_addr)
+  %l = load i16, ptr @g
+  ret i16 %l
+}
+