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

Haopeng Liu via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 21 12:41:02 PST 2024


https://github.com/haopliu updated https://github.com/llvm/llvm-project/pull/120044

>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 1/4] 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
+}
+

>From fa652d9e8186d4a82411ccf635ab7a76779d5ae6 Mon Sep 17 00:00:00 2001
From: Haopeng Liu <haopliu at google.com>
Date: Sat, 21 Dec 2024 20:21:40 +0000
Subject: [PATCH 2/4] Check whether an arg can from Alloca that doesn't escape

---
 .../Scalar/DeadStoreElimination.cpp           | 51 ++++++++++++++++---
 .../DeadStoreElimination/inter-procedural.ll  | 33 ++++--------
 2 files changed, 53 insertions(+), 31 deletions(-)

diff --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
index 19f6bba583941b..315fab3b53e94a 100644
--- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
@@ -1665,16 +1665,11 @@ 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 (auto *CB = dyn_cast<CallBase>(UseInst))
-          IsKillingDefFromInitAttr =
-              KillingI == UseInst &&
-              KillingUndObj == getUnderlyingObject(MaybeDeadLoc.Ptr) &&
-              CB->onlyAccessesInaccessibleMemOrArgMem();
+        if (KillingI == UseInst &&
+            KillingUndObj == getUnderlyingObject(MaybeDeadLoc.Ptr))
+          IsKillingDefFromInitAttr = true;
       }
 
       if (isReadClobber(MaybeDeadLoc, UseInst) && !IsKillingDefFromInitAttr) {
@@ -2266,6 +2261,39 @@ struct DSEState {
   bool eliminateDeadDefs(const MemoryDefWrapper &KillingDefWrapper);
 };
 
+// Return true if "Arg" is an Alloca or GEP from Alloca, and the alloca ptr
+// doesn't escape.
+bool ValidFromAlloca(Value *Arg) {
+  const auto *AI = dyn_cast<AllocaInst>(Arg);
+  const auto *GEP = dyn_cast<GetElementPtrInst>(Arg);
+  if (!AI) {
+    if (!GEP || !dyn_cast<AllocaInst>(GEP->getPointerOperand()))
+      return false;
+  }
+
+  // No need for a visited set as we don't look through phis.
+  SmallVector<Use *, 4> Worklist;
+  for (Use &U : Arg->uses())
+    Worklist.push_back(&U);
+
+  while (!Worklist.empty()) {
+    Use *U = Worklist.pop_back_val();
+    Instruction *I = cast<Instruction>(U->getUser());
+
+    if (auto *GEP = dyn_cast<GetElementPtrInst>(I)) {
+      for (Use &U : GEP->uses())
+        Worklist.push_back(&U);
+    } else if (auto *CB = dyn_cast<CallBase>(I)) {
+      if (CB->isArgOperand(U)) {
+        unsigned ArgNo = CB->getArgOperandNo(U);
+        if (!CB->paramHasAttr(ArgNo, Attribute::NoCapture))
+          return false;
+      }
+    }
+  }
+  return true;
+}
+
 SmallVector<MemoryLocation, 1>
 DSEState::getInitializesArgMemLoc(const Instruction *I) {
   const CallBase *CB = dyn_cast<CallBase>(I);
@@ -2281,6 +2309,13 @@ DSEState::getInitializesArgMemLoc(const Instruction *I) {
       Inits = InitializesAttr.getValueAsConstantRangeList();
 
     Value *CurArg = CB->getArgOperand(Idx);
+    // Check whether "CurArg" could alias with global variables. We require
+    // either it's an Alloca that doesn't escape or the "CB" only accesses arg
+    // or inaccessible mem.
+    if (!Inits.empty() && !ValidFromAlloca(CurArg) &&
+        !CB->onlyAccessesInaccessibleMemOrArgMem())
+      Inits = ConstantRangeList();
+
     // We don't perform incorrect DSE on unwind edges in the current function,
     // and use the "initializes" attribute to kill dead stores if:
     // - The call does not throw exceptions, "CB->doesNotThrow()".
diff --git a/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll b/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll
index 9c040d4d5bb380..a17a1071e9820f 100644
--- a/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll
@@ -2,22 +2,12 @@
 ; 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() {
@@ -225,25 +215,23 @@ 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() {
 ; CHECK-LABEL: @large_p1_caller(
-; CHECK-NEXT:    [[PTR:%.*]] = alloca [200 x i8], align 1
-; CHECK-NEXT:    call void @large_p1(ptr [[PTR]])
-; CHECK-NEXT:    [[L:%.*]] = load i16, ptr [[PTR]], align 2
+; CHECK-NEXT:    [[PTR:%.*]] = alloca [300 x i8], align 1
+; CHECK-NEXT:    [[TMP:%.*]] = getelementptr i8, ptr [[PTR]], i64 100
+; CHECK-NEXT:    call void @large_p1(ptr [[TMP]])
+; CHECK-NEXT:    [[L:%.*]] = load i16, ptr [[TMP]], align 2
 ; CHECK-NEXT:    ret i16 [[L]]
 ;
-  %ptr = alloca [200 x i8]
-  call void @llvm.memset.p0.i64(ptr %ptr, i8 42, i64 100, i1 false)
-  call void @large_p1(ptr %ptr)
-  %l = load i16, ptr %ptr
+  %ptr = alloca [300 x i8]
+  %tmp = getelementptr i8, ptr %ptr, i64 100
+  call void @llvm.memset.p0.i64(ptr %tmp, i8 42, i64 100, i1 false)
+  call void @large_p1(ptr %tmp)
+  %l = load i16, ptr %tmp
   ret i16 %l
 }
 
@@ -315,8 +303,7 @@ define i16 @large_p2_may_or_partial_alias_caller2(ptr %base1, ptr %base2) {
 
 @g = global i16 123, align 2
 
-declare void @read_global(ptr nocapture noundef initializes((0, 2)))
-  memory(read, argmem: write, inaccessiblemem: none) nounwind
+declare void @read_global(ptr nocapture noundef initializes((0, 2))) nounwind
 
 define i16 @global_var_alias() {
 ; CHECK-LABEL: @global_var_alias(

>From defcdb15c9426a1bfc9bc4c789092c4f0b4ecaf1 Mon Sep 17 00:00:00 2001
From: Haopeng Liu <haopliu at google.com>
Date: Sat, 21 Dec 2024 20:25:41 +0000
Subject: [PATCH 3/4] Fix a unit test mistake

---
 .../Transforms/DeadStoreElimination/inter-procedural.ll   | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll b/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll
index a17a1071e9820f..7ca0cf6157403f 100644
--- a/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll
@@ -307,14 +307,14 @@ declare void @read_global(ptr nocapture noundef initializes((0, 2))) 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:    store i16 0, ptr @g, align 4
+; CHECK-NEXT:    [[G_ADDR:%.*]] = getelementptr i8, 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
+  store i16 0, ptr @g, align 4
+  %g_addr = getelementptr i8, ptr @g, i64 1
   call void @read_global(ptr %g_addr)
   %l = load i16, ptr @g
   ret i16 %l

>From 1ff8ebc1154f11f0f01c7f5ef82879877b0b18d1 Mon Sep 17 00:00:00 2001
From: Haopeng Liu <haopliu at google.com>
Date: Sat, 21 Dec 2024 20:39:51 +0000
Subject: [PATCH 4/4] Add an unit test about alloca then escape

---
 .../DeadStoreElimination/inter-procedural.ll  | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll b/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll
index 7ca0cf6157403f..f09735e7b2dd59 100644
--- a/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll
+++ b/llvm/test/Transforms/DeadStoreElimination/inter-procedural.ll
@@ -39,6 +39,26 @@ define i16 @p1_write_then_read_caller() {
   ret i16 %l
 }
 
+declare void @fn_capture(ptr noundef)
+; Function Attrs: mustprogress nounwind uwtable
+define i16 @p1_write_then_read_caller_escape() {
+; CHECK-LABEL: @p1_write_then_read_caller_escape(
+; CHECK-NEXT:    [[PTR:%.*]] = alloca i16, align 2
+; CHECK-NEXT:    store i16 0, ptr [[PTR]], align 2
+; CHECK-NEXT:    call void @fn_capture(ptr [[PTR]])
+; CHECK-NEXT:    call void @p1_write_then_read(ptr [[PTR]])
+; CHECK-NEXT:    [[L:%.*]] = load i16, ptr [[PTR]], align 2
+; CHECK-NEXT:    ret i16 [[L]]
+;
+  %ptr = alloca i16
+  store i16 0, ptr %ptr
+  call void @fn_capture(ptr %ptr)
+  call void @p1_write_then_read(ptr %ptr)
+  %l = load i16, ptr %ptr
+  ret i16 %l
+}
+
+
 ; Function Attrs: mustprogress nounwind uwtable
 define i16 @p1_write_then_read_caller_with_clobber() {
 ; CHECK-LABEL: @p1_write_then_read_caller_with_clobber(
@@ -304,6 +324,7 @@ define i16 @large_p2_may_or_partial_alias_caller2(ptr %base1, ptr %base2) {
 @g = global i16 123, align 2
 
 declare void @read_global(ptr nocapture noundef initializes((0, 2))) nounwind
+  memory(read, argmem: write, inaccessiblemem: none) nounwind
 
 define i16 @global_var_alias() {
 ; CHECK-LABEL: @global_var_alias(



More information about the llvm-commits mailing list