Revert "[Coroutines] Salvage the debug information for coroutine frames within optimizations"

This reverts commit 522c253.

This series of commits causes Clang crashes. The reproducer is posted on
08a0dec.

Author: gribozavr

clang/test/CodeGenCoroutines/coro-dwarf-O2.cpp

@@ -1914,7 +1914,8 @@ static void insertSpills(const FrameDataInfo &FrameData, coro::Shape &Shape) {
           }
           // This dbg.declare is for the main function entry point.  It
           // will be deleted in all coro-split functions.
-          coro::salvageDebugInfo(ArgToAllocaMap, *DDI, false /*UseEntryValue*/);
+          coro::salvageDebugInfo(ArgToAllocaMap, *DDI, Shape.OptimizeFrame,
+                                 false /*UseEntryValue*/);
         };
         for_each(DIs, SalvageOne);
         for_each(DVRs, SalvageOne);
@@ -2851,8 +2852,9 @@ static void collectFrameAlloca(AllocaInst *AI, coro::Shape &Shape,
 
 static std::optional<std::pair<Value &, DIExpression &>>
 salvageDebugInfoImpl(SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
-                     bool UseEntryValue, Function *F, Value *Storage,
-                     DIExpression *Expr, bool SkipOutermostLoad) {
+                     bool OptimizeFrame, bool UseEntryValue, Function *F,
+                     Value *Storage, DIExpression *Expr,
+                     bool SkipOutermostLoad) {
   IRBuilder<> Builder(F->getContext());
   auto InsertPt = F->getEntryBlock().getFirstInsertionPt();
   while (isa<IntrinsicInst>(InsertPt))
@@ -2904,9 +2906,10 @@ salvageDebugInfoImpl(SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
 
   // If the coroutine frame is an Argument, store it in an alloca to improve
   // its availability (e.g. registers may be clobbered).
-  // Avoid this if the value is guaranteed to be available through other means
-  // (e.g. swift ABI guarantees).
-  if (StorageAsArg && !IsSwiftAsyncArg) {
+  // Avoid this if optimizations are enabled (they would remove the alloca) or
+  // if the value is guaranteed to be available through other means (e.g. swift
+  // ABI guarantees).
+  if (StorageAsArg && !OptimizeFrame && !IsSwiftAsyncArg) {
     auto &Cached = ArgToAllocaMap[StorageAsArg];
     if (!Cached) {
       Cached = Builder.CreateAlloca(Storage->getType(), 0, nullptr,
@@ -2929,17 +2932,17 @@ salvageDebugInfoImpl(SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
 
 void coro::salvageDebugInfo(
     SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
-    DbgVariableIntrinsic &DVI, bool UseEntryValue) {
+    DbgVariableIntrinsic &DVI, bool OptimizeFrame, bool UseEntryValue) {
 
   Function *F = DVI.getFunction();
   // Follow the pointer arithmetic all the way to the incoming
   // function argument and convert into a DIExpression.
   bool SkipOutermostLoad = !isa<DbgValueInst>(DVI);
   Value *OriginalStorage = DVI.getVariableLocationOp(0);
 
-  auto SalvagedInfo =
-      ::salvageDebugInfoImpl(ArgToAllocaMap, UseEntryValue, F, OriginalStorage,
-                             DVI.getExpression(), SkipOutermostLoad);
+  auto SalvagedInfo = ::salvageDebugInfoImpl(
+      ArgToAllocaMap, OptimizeFrame, UseEntryValue, F, OriginalStorage,
+      DVI.getExpression(), SkipOutermostLoad);
   if (!SalvagedInfo)
     return;
 
@@ -2971,17 +2974,17 @@ void coro::salvageDebugInfo(
 
 void coro::salvageDebugInfo(
     SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
-    DbgVariableRecord &DVR, bool UseEntryValue) {
+    DbgVariableRecord &DVR, bool OptimizeFrame, bool UseEntryValue) {
 
   Function *F = DVR.getFunction();
   // Follow the pointer arithmetic all the way to the incoming
   // function argument and convert into a DIExpression.
   bool SkipOutermostLoad = DVR.isDbgDeclare();
   Value *OriginalStorage = DVR.getVariableLocationOp(0);
 
-  auto SalvagedInfo =
-      ::salvageDebugInfoImpl(ArgToAllocaMap, UseEntryValue, F, OriginalStorage,
-                             DVR.getExpression(), SkipOutermostLoad);
+  auto SalvagedInfo = ::salvageDebugInfoImpl(
+      ArgToAllocaMap, OptimizeFrame, UseEntryValue, F, OriginalStorage,
+      DVR.getExpression(), SkipOutermostLoad);
   if (!SalvagedInfo)
     return;
 

llvm/lib/Transforms/Coroutines/CoroFrame.cpp

@@ -29,14 +29,14 @@ void replaceCoroFree(CoroIdInst *CoroId, bool Elide);
 /// Attempts to rewrite the location operand of debug intrinsics in terms of
 /// the coroutine frame pointer, folding pointer offsets into the DIExpression
 /// of the intrinsic.
-/// If the frame pointer is an Argument, store it into an alloca to enhance the
-/// debugability.
+/// If the frame pointer is an Argument, store it into an alloca if
+/// OptimizeFrame is false.
 void salvageDebugInfo(
     SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
-    DbgVariableIntrinsic &DVI, bool IsEntryPoint);
+    DbgVariableIntrinsic &DVI, bool OptimizeFrame, bool IsEntryPoint);
 void salvageDebugInfo(
     SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
-    DbgVariableRecord &DVR, bool UseEntryValue);
+    DbgVariableRecord &DVR, bool OptimizeFrame, bool UseEntryValue);
 
 // Keeps data and helper functions for lowering coroutine intrinsics.
 struct LowererBase {

llvm/lib/Transforms/Coroutines/CoroInternal.h

@@ -735,9 +735,11 @@ void CoroCloner::salvageDebugInfo() {
   bool UseEntryValue =
       llvm::Triple(OrigF.getParent()->getTargetTriple()).isArch64Bit();
   for (DbgVariableIntrinsic *DVI : Worklist)
-    coro::salvageDebugInfo(ArgToAllocaMap, *DVI, UseEntryValue);
+    coro::salvageDebugInfo(ArgToAllocaMap, *DVI, Shape.OptimizeFrame,
+                           UseEntryValue);
   for (DbgVariableRecord *DVR : DbgVariableRecords)
-    coro::salvageDebugInfo(ArgToAllocaMap, *DVR, UseEntryValue);
+    coro::salvageDebugInfo(ArgToAllocaMap, *DVR, Shape.OptimizeFrame,
+                           UseEntryValue);
 
   // Remove all salvaged dbg.declare intrinsics that became
   // either unreachable or stale due to the CoroSplit transformation.
@@ -1960,9 +1962,11 @@ splitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
   SmallDenseMap<Argument *, AllocaInst *, 4> ArgToAllocaMap;
   auto [DbgInsts, DbgVariableRecords] = collectDbgVariableIntrinsics(F);
   for (auto *DDI : DbgInsts)
-    coro::salvageDebugInfo(ArgToAllocaMap, *DDI, false /*UseEntryValue*/);
+    coro::salvageDebugInfo(ArgToAllocaMap, *DDI, Shape.OptimizeFrame,
+                           false /*UseEntryValue*/);
   for (DbgVariableRecord *DVR : DbgVariableRecords)
-    coro::salvageDebugInfo(ArgToAllocaMap, *DVR, false /*UseEntryValue*/);
+    coro::salvageDebugInfo(ArgToAllocaMap, *DVR, Shape.OptimizeFrame,
+                           false /*UseEntryValue*/);
   return Shape;
 }
 

llvm/lib/Transforms/Coroutines/CoroSplit.cpp

@@ -1,14 +1,12 @@
 ; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split<reuse-storage>),function(sroa)' -S | FileCheck %s
 ; RUN: opt --try-experimental-debuginfo-iterators < %s -passes='module(coro-early),cgscc(coro-split<reuse-storage>),function(sroa)' -S | FileCheck %s
 
-; Checks the dbg informations about promise and coroutine frames under O2.
+; Checks whether the dbg.declare for `__promise` remains valid under O2.
 
 ; CHECK-LABEL: define internal fastcc void @f.resume({{.*}})
 ; CHECK:       entry.resume:
-; CHECK:        #dbg_value(ptr poison, ![[PROMISEVAR_RESUME:[0-9]+]], !DIExpression(DW_OP_deref, DW_OP_plus_uconst, 16
-; CHECK:        #dbg_value(ptr %begin, ![[CORO_FRAME:[0-9]+]], !DIExpression(DW_OP_deref)
+; CHECK:        #dbg_declare(ptr %begin, ![[PROMISEVAR_RESUME:[0-9]+]], !DIExpression(
 ;
-; CHECK: ![[CORO_FRAME]] = !DILocalVariable(name: "__coro_frame"
 ; CHECK: ![[PROMISEVAR_RESUME]] = !DILocalVariable(name: "__promise"
 %promise_type = type { i32, i32, double }