[CoroSplit] Always erase lifetime intrinsics for spilled allocas

[NewSigma]

If the control flow between lifetime.start and lifetime.end is too complex, it is acceptable to give up the optimization opportunity and collect the alloca to the frame. However, storing to the frame will lengthen the lifetime of the alloca, and the sanitizer will complain. I propose we always erase lifetime intrinsics of spilled allocas.

Fix #124612

[llvmbot]

@llvm/pr-subscribers-coroutines

Author: Weibo He (NewSigma)

Changes

If the control flow between lifetime.start and lifetime.end is too complex, it is acceptable to give up the optimization opportunity and collect the alloca to the frame. However, storing to the frame will lengthen the lifetime of the alloca, and the sanitizer will complain. I propose we always erase lifetime intrinsics of spilled allocas.

Fix #124612

---

Full diff: https://github.com/llvm/llvm-project/pull/142551.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/Coroutines/CoroFrame.cpp (+14-11)

diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 2f5f1089067bf..fdfddcc3425e9 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -1212,14 +1212,26 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
   Builder.SetInsertPoint(Shape.AllocaSpillBlock,
                          Shape.AllocaSpillBlock->begin());
   SmallVector<Instruction *, 4> UsersToUpdate;
+  SmallVector<Instruction *, 4> Lifetimes;
   for (const auto &A : FrameData.Allocas) {
     AllocaInst *Alloca = A.Alloca;
     UsersToUpdate.clear();
+    Lifetimes.clear();
     for (User *U : Alloca->users()) {
       auto *I = cast<Instruction>(U);
-      if (DT.dominates(Shape.CoroBegin, I))
+      if (I->isLifetimeStartOrEnd())
+        Lifetimes.push_back(I);
+      else if (DT.dominates(Shape.CoroBegin, I))
         UsersToUpdate.push_back(I);
     }
+
+    // If it is hard to analyze, we will give up and put allocas to frame,
+    // even if they never cross suspend points.
+    // Lifetime intrinsics referring to alloca may fail guard storing to frame.
+    // Lifetime intrinsics referring to frames may block further optimizations.
+    for (auto *I : Lifetimes)
+      I->eraseFromParent();
+
     if (UsersToUpdate.empty())
       continue;
     auto *G = GetFramePointer(Alloca);
@@ -1233,17 +1245,8 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
     for (auto *DVR : DbgVariableRecords)
       DVR->replaceVariableLocationOp(Alloca, G);
 
-    for (Instruction *I : UsersToUpdate) {
-      // It is meaningless to retain the lifetime intrinsics refer for the
-      // member of coroutine frames and the meaningless lifetime intrinsics
-      // are possible to block further optimizations.
-      if (I->isLifetimeStartOrEnd()) {
-        I->eraseFromParent();
-        continue;
-      }
-
+    for (Instruction *I : UsersToUpdate)
       I->replaceUsesOfWith(Alloca, G);
-    }
   }
   Builder.SetInsertPoint(&*Shape.getInsertPtAfterFramePtr());
   for (const auto &A : FrameData.Allocas) {

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Weibo He (NewSigma)

Changes

If the control flow between lifetime.start and lifetime.end is too complex, it is acceptable to give up the optimization opportunity and collect the alloca to the frame. However, storing to the frame will lengthen the lifetime of the alloca, and the sanitizer will complain. I propose we always erase lifetime intrinsics of spilled allocas.

Fix #124612

---

Full diff: https://github.com/llvm/llvm-project/pull/142551.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/Coroutines/CoroFrame.cpp (+14-11)

diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 2f5f1089067bf..fdfddcc3425e9 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -1212,14 +1212,26 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
   Builder.SetInsertPoint(Shape.AllocaSpillBlock,
                          Shape.AllocaSpillBlock->begin());
   SmallVector<Instruction *, 4> UsersToUpdate;
+  SmallVector<Instruction *, 4> Lifetimes;
   for (const auto &A : FrameData.Allocas) {
     AllocaInst *Alloca = A.Alloca;
     UsersToUpdate.clear();
+    Lifetimes.clear();
     for (User *U : Alloca->users()) {
       auto *I = cast<Instruction>(U);
-      if (DT.dominates(Shape.CoroBegin, I))
+      if (I->isLifetimeStartOrEnd())
+        Lifetimes.push_back(I);
+      else if (DT.dominates(Shape.CoroBegin, I))
         UsersToUpdate.push_back(I);
     }
+
+    // If it is hard to analyze, we will give up and put allocas to frame,
+    // even if they never cross suspend points.
+    // Lifetime intrinsics referring to alloca may fail guard storing to frame.
+    // Lifetime intrinsics referring to frames may block further optimizations.
+    for (auto *I : Lifetimes)
+      I->eraseFromParent();
+
     if (UsersToUpdate.empty())
       continue;
     auto *G = GetFramePointer(Alloca);
@@ -1233,17 +1245,8 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
     for (auto *DVR : DbgVariableRecords)
       DVR->replaceVariableLocationOp(Alloca, G);
 
-    for (Instruction *I : UsersToUpdate) {
-      // It is meaningless to retain the lifetime intrinsics refer for the
-      // member of coroutine frames and the meaningless lifetime intrinsics
-      // are possible to block further optimizations.
-      if (I->isLifetimeStartOrEnd()) {
-        I->eraseFromParent();
-        continue;
-      }
-
+    for (Instruction *I : UsersToUpdate)
       I->replaceUsesOfWith(Alloca, G);
-    }
   }
   Builder.SetInsertPoint(&*Shape.getInsertPtAfterFramePtr());
   for (const auto &A : FrameData.Allocas) {

[ChuanqiXu9]

LGTM