[clang] 07514fa - [Coroutines] Salvage the debug information for coroutine frames within optimizations
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 28 02:14:36 PDT 2024
Author: Chuanqi Xu
Date: 2024-08-28T17:02:12+08:00
New Revision: 07514fa9b607fd80a72a80270d714e22d842fa39
URL: https://github.com/llvm/llvm-project/commit/07514fa9b607fd80a72a80270d714e22d842fa39
DIFF: https://github.com/llvm/llvm-project/commit/07514fa9b607fd80a72a80270d714e22d842fa39.diff
LOG: [Coroutines] Salvage the debug information for coroutine frames within optimizations
This patch tries to salvage the debug information for the coroutine
frames within optimizations by creating the help alloca varaibles with
optimizations too. We didn't do this when I implement it initially. I
roughtly remember the reason was, we feel the additional help alloca
variable may pessimize the performance, which is almost the most
important thing under optimizations. But now, it looks like the new
inserted help alloca variables can be optimized out by the following
optimizations. So it looks like the time to make it available within
optimizations.
And also, it looks like the following optimizations will convert the
generated dbg.declare instrinsic into dbg.value intrinsic within
optimizations.
In LLVM's test, there is a slightly regression
that a dbg.declare for the promise object failed to be remained after
this change. But it looks like we won't have a chance to see dbg.declare
for the promise object when we split the coroutine as that dbg.declare
will be converted into a dbg.value in early stage.
So everything looks fine.
Added:
clang/test/CodeGenCoroutines/coro-dwarf-O2.cpp
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:
################################################################################
diff --git a/clang/test/CodeGenCoroutines/coro-dwarf-O2.cpp b/clang/test/CodeGenCoroutines/coro-dwarf-O2.cpp
new file mode 100644
index 00000000000000..53f4a07982e427
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-dwarf-O2.cpp
@@ -0,0 +1,39 @@
+// 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 740ae661359445..f76cfe01b58cfd 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -1947,8 +1947,7 @@ 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, Shape.OptimizeFrame,
- false /*UseEntryValue*/);
+ coro::salvageDebugInfo(ArgToAllocaMap, *DDI, false /*UseEntryValue*/);
};
for_each(DIs, SalvageOne);
for_each(DVRs, SalvageOne);
@@ -2885,9 +2884,8 @@ static void collectFrameAlloca(AllocaInst *AI, coro::Shape &Shape,
static std::optional<std::pair<Value &, DIExpression &>>
salvageDebugInfoImpl(SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
- bool OptimizeFrame, bool UseEntryValue, Function *F,
- Value *Storage, DIExpression *Expr,
- bool SkipOutermostLoad) {
+ bool UseEntryValue, Function *F, Value *Storage,
+ DIExpression *Expr, bool SkipOutermostLoad) {
IRBuilder<> Builder(F->getContext());
auto InsertPt = F->getEntryBlock().getFirstInsertionPt();
while (isa<IntrinsicInst>(InsertPt))
@@ -2939,10 +2937,9 @@ 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 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) {
+ // Avoid this if the value is guaranteed to be available through other means
+ // (e.g. swift ABI guarantees).
+ if (StorageAsArg && !IsSwiftAsyncArg) {
auto &Cached = ArgToAllocaMap[StorageAsArg];
if (!Cached) {
Cached = Builder.CreateAlloca(Storage->getType(), 0, nullptr,
@@ -2965,7 +2962,7 @@ salvageDebugInfoImpl(SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
void coro::salvageDebugInfo(
SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
- DbgVariableIntrinsic &DVI, bool OptimizeFrame, bool UseEntryValue) {
+ DbgVariableIntrinsic &DVI, bool UseEntryValue) {
Function *F = DVI.getFunction();
// Follow the pointer arithmetic all the way to the incoming
@@ -2973,9 +2970,9 @@ void coro::salvageDebugInfo(
bool SkipOutermostLoad = !isa<DbgValueInst>(DVI);
Value *OriginalStorage = DVI.getVariableLocationOp(0);
- auto SalvagedInfo = ::salvageDebugInfoImpl(
- ArgToAllocaMap, OptimizeFrame, UseEntryValue, F, OriginalStorage,
- DVI.getExpression(), SkipOutermostLoad);
+ auto SalvagedInfo =
+ ::salvageDebugInfoImpl(ArgToAllocaMap, UseEntryValue, F, OriginalStorage,
+ DVI.getExpression(), SkipOutermostLoad);
if (!SalvagedInfo)
return;
@@ -3007,7 +3004,7 @@ void coro::salvageDebugInfo(
void coro::salvageDebugInfo(
SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
- DbgVariableRecord &DVR, bool OptimizeFrame, bool UseEntryValue) {
+ DbgVariableRecord &DVR, bool UseEntryValue) {
Function *F = DVR.getFunction();
// Follow the pointer arithmetic all the way to the incoming
@@ -3015,9 +3012,9 @@ void coro::salvageDebugInfo(
bool SkipOutermostLoad = DVR.isDbgDeclare();
Value *OriginalStorage = DVR.getVariableLocationOp(0);
- auto SalvagedInfo = ::salvageDebugInfoImpl(
- ArgToAllocaMap, OptimizeFrame, UseEntryValue, F, OriginalStorage,
- DVR.getExpression(), SkipOutermostLoad);
+ auto SalvagedInfo =
+ ::salvageDebugInfoImpl(ArgToAllocaMap, 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 5716fd0ea4ab96..d535ad7f85d74a 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 if
-/// OptimizeFrame is false.
+/// If the frame pointer is an Argument, store it into an alloca to enhance the
+/// debugability.
void salvageDebugInfo(
SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
- DbgVariableIntrinsic &DVI, bool OptimizeFrame, bool IsEntryPoint);
+ DbgVariableIntrinsic &DVI, bool IsEntryPoint);
void salvageDebugInfo(
SmallDenseMap<Argument *, AllocaInst *, 4> &ArgToAllocaMap,
- DbgVariableRecord &DVR, bool OptimizeFrame, bool UseEntryValue);
+ DbgVariableRecord &DVR, 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 fab387c65ab41f..6bf3c75b95113e 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -735,11 +735,9 @@ void CoroCloner::salvageDebugInfo() {
bool UseEntryValue =
llvm::Triple(OrigF.getParent()->getTargetTriple()).isArch64Bit();
for (DbgVariableIntrinsic *DVI : Worklist)
- coro::salvageDebugInfo(ArgToAllocaMap, *DVI, Shape.OptimizeFrame,
- UseEntryValue);
+ coro::salvageDebugInfo(ArgToAllocaMap, *DVI, UseEntryValue);
for (DbgVariableRecord *DVR : DbgVariableRecords)
- coro::salvageDebugInfo(ArgToAllocaMap, *DVR, Shape.OptimizeFrame,
- UseEntryValue);
+ coro::salvageDebugInfo(ArgToAllocaMap, *DVR, UseEntryValue);
// Remove all salvaged dbg.declare intrinsics that became
// either unreachable or stale due to the CoroSplit transformation.
@@ -1961,11 +1959,9 @@ splitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
SmallDenseMap<Argument *, AllocaInst *, 4> ArgToAllocaMap;
auto [DbgInsts, DbgVariableRecords] = collectDbgVariableIntrinsics(F);
for (auto *DDI : DbgInsts)
- coro::salvageDebugInfo(ArgToAllocaMap, *DDI, Shape.OptimizeFrame,
- false /*UseEntryValue*/);
+ coro::salvageDebugInfo(ArgToAllocaMap, *DDI, false /*UseEntryValue*/);
for (DbgVariableRecord *DVR : DbgVariableRecords)
- coro::salvageDebugInfo(ArgToAllocaMap, *DVR, Shape.OptimizeFrame,
- false /*UseEntryValue*/);
+ coro::salvageDebugInfo(ArgToAllocaMap, *DVR, 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 7ffa2ac153c853..588f47959cc5d5 100644
--- a/llvm/test/Transforms/Coroutines/coro-debug-O2.ll
+++ b/llvm/test/Transforms/Coroutines/coro-debug-O2.ll
@@ -1,12 +1,14 @@
; 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 whether the dbg.declare for `__promise` remains valid under O2.
+; Checks the dbg informations about promise and coroutine frames under O2.
; CHECK-LABEL: define internal fastcc void @f.resume({{.*}})
; CHECK: entry.resume:
-; CHECK: #dbg_declare(ptr %begin, ![[PROMISEVAR_RESUME:[0-9]+]], !DIExpression(
+; 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: ![[CORO_FRAME]] = !DILocalVariable(name: "__coro_frame"
; CHECK: ![[PROMISEVAR_RESUME]] = !DILocalVariable(name: "__promise"
%promise_type = type { i32, i32, double }
More information about the cfe-commits
mailing list