[Coroutines] Directly remove unnecessary lifetime intrinsics

[nikic]

Summary

The insertSpills() code will currently skip lifetime intrinsic users when replacing the alloca with a frame reference. Rather than leaving behind the dead lifetime intrinsics working on the old alloca, directly remove them. This makes sure the alloca can be dropped as well.

I noticed this as a regression when converting tests to opaque pointers. Without opaque pointers, this code didn't really do anything, because there would usually be a bitcast in between. The lifetimes would get rewritten to the frame pointer. With opaque pointers, this code now triggers and leaves behind users of the old allocas.

I'm also open to just dropping this check -- I'm trusting the comment that dropping the lifetime intrinsics is indeed what we want.

[ChuanqiXu]

LGTM. Thanks!

clang/test/CodeGenCoroutines/coro-always-inline.cpp

struct promise_type {
     T get_return_object() { return T(); }
     void unhandled_exception() {}
   };
 };
 } // namespace std
 
 // CHECK-LABEL: @_Z3foov
 // CHECK-LABEL: entry:
-// CHECK: call void @llvm.lifetime.start.p0(i64 1, ptr %ref.tmp{{.*}})
-// CHECK: call void @llvm.lifetime.end.p0(i64 1, ptr %ref.tmp{{.*}})
-
-// CHECK: call void @llvm.lifetime.start.p0(i64 1, ptr %ref.tmp{{.*}})
-// CHECK: call void @llvm.lifetime.end.p0(i64 1, ptr %ref.tmp{{.*}})
+// CHECK: %ref.tmp.reload.addr = getelementptr
+// CHECK: %ref.tmp4.reload.addr = getelementptr
 void foo() { co_return; }
 
 // Check that bar is not inlined even it's marked as always_inline.
 
 // CHECK-LABEL:   define {{.*}} void @_Z3bazv()
 // CHECK:         call void @_Z3barv(
 __attribute__((__always_inline__)) void bar() {
   co_return;
 }
 void baz() {
   bar();
   co_return;
 }

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

for (const auto &A : FrameData.Allocas) {
     G->setName(Alloca->getName() + Twine(".reload.addr"));
 
     SmallVector<DbgVariableIntrinsic *, 4> DIs;
     findDbgUsers(DIs, Alloca);
     for (auto *DVI : DIs)
       DVI->replaceUsesOfWith(Alloca, G);
 
     for (Instruction *I : UsersToUpdate) {
-      // It is meaningless to remain the lifetime intrinsics refer for the
+      // 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())
+      if (I->isLifetimeStartOrEnd()) {
+        I->eraseFromParent();
         continue;
+      }
 
       I->replaceUsesOfWith(Alloca, G);
     }
   }
   Builder.SetInsertPoint(Shape.getInsertPtAfterFramePtr());
   for (const auto &A : FrameData.Allocas) {
     AllocaInst *Alloca = A.Alloca;
     if (A.MayWriteBeforeCoroBegin) {

llvm/test/Transforms/Coroutines/coro-alloca-loop-carried-address.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes='cgscc(coro-split),simplifycfg,early-cse' -S | FileCheck %s
 
 @escape_hatch0 = external global i64
 @escape_hatch1 = external global i64
 
 define void @foo() presplitcoroutine {
 ; CHECK-LABEL: @foo(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[STACKVAR0:%.*]] = alloca i64, align 8
 ; CHECK-NEXT:    [[ID:%.*]] = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr @foo.resumers)
 ; CHECK-NEXT:    [[ALLOC:%.*]] = call ptr @malloc(i64 40)
 ; CHECK-NEXT:    [[VFRAME:%.*]] = call noalias nonnull ptr @llvm.coro.begin(token [[ID]], ptr [[ALLOC]])
 ; CHECK-NEXT:    store ptr @foo.resume, ptr [[VFRAME]], align 8
 ; CHECK-NEXT:    [[DESTROY_ADDR:%.*]] = getelementptr inbounds [[FOO_FRAME:%.*]], ptr [[VFRAME]], i32 0, i32 1
 ; CHECK-NEXT:    store ptr @foo.destroy, ptr [[DESTROY_ADDR]], align 8
 ; CHECK-NEXT:    [[STACKVAR0_RELOAD_ADDR:%.*]] = getelementptr inbounds [[FOO_FRAME]], ptr [[VFRAME]], i32 0, i32 2
 ; CHECK-NEXT:    [[STACKVAR1_RELOAD_ADDR:%.*]] = getelementptr inbounds [[FOO_FRAME]], ptr [[VFRAME]], i32 0, i32 3