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

Dmitri Gribenko via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 21 14:57:19 PDT 2024


Author: Dmitri Gribenko
Date: 2024-08-21T23:49:45+02:00
New Revision: f709cd5add0ea36bb14259e9716bd74e5c762128

URL: https://github.com/llvm/llvm-project/commit/f709cd5add0ea36bb14259e9716bd74e5c762128
DIFF: https://github.com/llvm/llvm-project/commit/f709cd5add0ea36bb14259e9716bd74e5c762128.diff

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

This reverts commit 522c253f47ea27d8eeb759e06f8749092b1de71e.

This series of commits causes Clang crashes. The reproducer is posted on
https://github.com/llvm/llvm-project/commit/08a0dece2b2431db8abe650bb43cba01e781e1ce.

Added: 
    

Modified: 
    llvm/lib/Transforms/Coroutines/CoroFrame.cpp
    llvm/lib/Transforms/Coroutines/CoroInternal.h
    llvm/lib/Transforms/Coroutines/CoroSplit.cpp
    llvm/test/Transforms/Coroutines/coro-debug-O2.ll

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


################################################################################
diff  --git a/clang/test/CodeGenCoroutines/coro-dwarf-O2.cpp b/clang/test/CodeGenCoroutines/coro-dwarf-O2.cpp
deleted file mode 100644
index 53f4a07982e427..00000000000000
--- a/clang/test/CodeGenCoroutines/coro-dwarf-O2.cpp
+++ /dev/null
@@ -1,39 +0,0 @@
-// Check that we can still observe the value of the coroutine frame
-// with optimizations.
-//
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 \
-// RUN:   -emit-llvm %s -debug-info-kind=limited -dwarf-version=5 \
-// RUN:   -O2 -o - | FileCheck %s
-
-#include "Inputs/coroutine.h"
-
-template <>
-struct std::coroutine_traits<void> {
-  struct promise_type {
-    void get_return_object();
-    std::suspend_always initial_suspend();
-    std::suspend_always final_suspend() noexcept;
-    void return_void();
-    void unhandled_exception();
-  };
-};
-
-struct ScalarAwaiter {
-  template <typename F> void await_suspend(F);
-  bool await_ready();
-  int await_resume();
-};
-
-extern "C" void UseScalar(int);
-
-extern "C" void f() {
-  UseScalar(co_await ScalarAwaiter{});
-
-  int Val = co_await ScalarAwaiter{};
-
-  co_await ScalarAwaiter{};
-}
-
-// CHECK: define {{.*}}@f.resume({{.*}} %[[ARG:.*]])
-// CHECK:  #dbg_value(ptr %[[ARG]], ![[CORO_NUM:[0-9]+]], !DIExpression(DW_OP_deref)
-// CHECK: ![[CORO_NUM]] = !DILocalVariable(name: "__coro_frame"

diff  --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 00f49b7bdce294..fa04735340406d 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.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,7 +2932,7 @@ 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
@@ -2937,9 +2940,9 @@ void coro::salvageDebugInfo(
   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,7 +2974,7 @@ 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
@@ -2979,9 +2982,9 @@ void coro::salvageDebugInfo(
   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;
 

diff  --git a/llvm/lib/Transforms/Coroutines/CoroInternal.h b/llvm/lib/Transforms/Coroutines/CoroInternal.h
index d535ad7f85d74a..5716fd0ea4ab96 100644
--- a/llvm/lib/Transforms/Coroutines/CoroInternal.h
+++ b/llvm/lib/Transforms/Coroutines/CoroInternal.h
@@ -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 {

diff  --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 40bc932c3e0eef..8eceaef59a1e1f 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -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;
 }
 

diff  --git a/llvm/test/Transforms/Coroutines/coro-debug-O2.ll b/llvm/test/Transforms/Coroutines/coro-debug-O2.ll
index 588f47959cc5d5..7ffa2ac153c853 100644
--- a/llvm/test/Transforms/Coroutines/coro-debug-O2.ll
+++ b/llvm/test/Transforms/Coroutines/coro-debug-O2.ll
@@ -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 }
 


        


More information about the llvm-commits mailing list