[clang] [clang-tools-extra] [llvm] [codegen] Emit cleanups for lifetime-extended temporaries when stmt-expr has control-flow (PR #80698)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 5 08:03:38 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-codegen
Author: Utkarsh Saxena (usx95)
<details>
<summary>Changes</summary>
Partially solves https://github.com/llvm/llvm-project/issues/63818 for control flow in braced aggregate initialisation.
The reason why we miss cleanups for temporaries in braced aggregate init is that the lifetime of these temporaries is extended, and their cleanup is scheduled (added to EHStack) after the full expression.
When such expressions have control flow (through `return` or `co_await`), we only emit the `EHStack` cleanups which does not contain the `LifetimeExtendedCleanupStack` cleanups.
In this PR, we forcefully emit `LifetimeExtendedCleanupStack` if they are present while emitting a branch.
(This still does not work for control flow in array-like list initialization expressions.)
---
Full diff: https://github.com/llvm/llvm-project/pull/80698.diff
4 Files Affected:
- (modified) clang/lib/CodeGen/CGCleanup.cpp (+22-9)
- (modified) clang/lib/CodeGen/CodeGenFunction.h (+4)
- (added) clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp (+37)
- (added) clang/test/CodeGenCoroutines/coro-suspend-in-agg-init.cpp (+82)
``````````diff
diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index f87caf050eeaa..da0528b271aa3 100644
--- a/clang/lib/CodeGen/CGCleanup.cpp
+++ b/clang/lib/CodeGen/CGCleanup.cpp
@@ -488,16 +488,11 @@ void CodeGenFunction::PopCleanupBlocks(
}
}
-/// Pops cleanup blocks until the given savepoint is reached, then add the
-/// cleanups from the given savepoint in the lifetime-extended cleanups stack.
-void CodeGenFunction::PopCleanupBlocks(
- EHScopeStack::stable_iterator Old, size_t OldLifetimeExtendedSize,
- std::initializer_list<llvm::Value **> ValuesToReload) {
- PopCleanupBlocks(Old, ValuesToReload);
-
- // Move our deferred cleanups onto the EH stack.
+/// Adds deferred lifetime-extended cleanups onto the EH stack.
+void CodeGenFunction::AddLifetimeExtendedCleanups(size_t OldLifetimeExtendedSize) {
for (size_t I = OldLifetimeExtendedSize,
- E = LifetimeExtendedCleanupStack.size(); I != E; /**/) {
+ E = LifetimeExtendedCleanupStack.size();
+ I != E;) {
// Alignment should be guaranteed by the vptrs in the individual cleanups.
assert((I % alignof(LifetimeExtendedCleanupHeader) == 0) &&
"misaligned cleanup stack entry");
@@ -519,6 +514,17 @@ void CodeGenFunction::PopCleanupBlocks(
I += sizeof(ActiveFlag);
}
}
+}
+
+/// Pops cleanup blocks until the given savepoint is reached, then add the
+/// cleanups from the given savepoint in the lifetime-extended cleanups stack.
+void CodeGenFunction::PopCleanupBlocks(
+ EHScopeStack::stable_iterator Old, size_t OldLifetimeExtendedSize,
+ std::initializer_list<llvm::Value **> ValuesToReload) {
+ PopCleanupBlocks(Old, ValuesToReload);
+
+ // Move our deferred cleanups onto the EH stack.
+ AddLifetimeExtendedCleanups(OldLifetimeExtendedSize);
LifetimeExtendedCleanupStack.resize(OldLifetimeExtendedSize);
}
@@ -1102,6 +1108,13 @@ void CodeGenFunction::EmitBranchThroughCleanup(JumpDest Dest) {
if (!HaveInsertPoint())
return;
+ // If we have lifetime-extended (LE) cleanups, then we must be emitting a
+ // branch within an expression. Emit all the LE cleanups by adding them to the
+ // EHStack. Do not remove them from lifetime-extended stack, they need to be
+ // emitted again after the expression completes.
+ RunCleanupsScope LifetimeExtendedCleanups(*this);
+ AddLifetimeExtendedCleanups(0);
+
// Create the branch.
llvm::BranchInst *BI = Builder.CreateBr(Dest.getBlock());
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 143ad64e8816b..ac55e84034bc6 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -1143,6 +1143,10 @@ class CodeGenFunction : public CodeGenTypeCache {
PopCleanupBlocks(EHScopeStack::stable_iterator OldCleanupStackSize,
std::initializer_list<llvm::Value **> ValuesToReload = {});
+ /// Adds lifetime-extended cleanups from the given position to the stack.
+ /// (does not remove the cleanups from lifetime extended stack).
+ void AddLifetimeExtendedCleanups(size_t OldLifetimeExtendedSize);
+
/// Takes the old cleanup stack size and emits the cleanup blocks
/// that have been added, then adds all lifetime-extended cleanups from
/// the given position to the stack.
diff --git a/clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp b/clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp
new file mode 100644
index 0000000000000..214becd81e61a
--- /dev/null
+++ b/clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+// Context: GH63818
+struct Printy {
+ ~Printy() { }
+};
+
+struct Printies {
+ const Printy &a;
+ const Printy &b;
+ ~Printies() {}
+};
+
+bool foo();
+
+void bar() {
+ Printies p2{
+ // CHECK: store ptr %ref.tmp
+ Printy(),
+ ({
+ if(foo()) {
+ // CHECK-LABEL: if.then:
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label %return
+ return;
+ }
+ // CHECK-LABEL: if.end:
+ // CHECK-NEXT: store ptr %ref.tmp1
+ Printy();
+ })};
+ // CHECK-NEXT: call void @_ZN8PrintiesD1Ev
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label %return
+ return;
+}
+
diff --git a/clang/test/CodeGenCoroutines/coro-suspend-in-agg-init.cpp b/clang/test/CodeGenCoroutines/coro-suspend-in-agg-init.cpp
new file mode 100644
index 0000000000000..362e212b7fba3
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-suspend-in-agg-init.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 --std=c++20 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+// Context: GH63818
+
+#include "Inputs/coroutine.h"
+
+struct coroutine {
+ struct promise_type;
+ std::coroutine_handle<promise_type> handle;
+};
+
+struct coroutine::promise_type {
+ coroutine get_return_object() {
+ return {std::coroutine_handle<promise_type>::from_promise(*this)};
+ }
+ std::suspend_never initial_suspend() noexcept { return {}; }
+ std::suspend_always final_suspend() noexcept { return {}; }
+ void return_void() {}
+ void unhandled_exception() {}
+};
+
+struct Printy { ~Printy(); };
+
+struct Printies {
+ const Printy &a;
+ const Printy &b;
+ const Printy &c;
+};
+
+struct Awaiter : std::suspend_always {
+ Printy await_resume() { return {}; }
+};
+
+// CHECK: define dso_local ptr @_Z5test1v()
+coroutine test1() {
+ // CHECK-NOT: @_ZN6PrintyD1Ev
+ Printies p1{
+ Printy(),
+ co_await Awaiter{},
+ // CHECK: await.cleanup:
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label %cleanup{{.*}}.from.await.cleanup
+ // CHECK-NOT: @_ZN6PrintyD1Ev
+
+ co_await Awaiter{}
+ // CHECK: await2.cleanup:
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label %cleanup{{.*}}.from.await2.cleanup
+ // CHECK-NOT: @_ZN6PrintyD1Ev
+ };
+
+ // CHECK-COUNT-3: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label
+
+ // CHECK-NOT: @_ZN6PrintyD1Ev
+
+ // CHECK: unreachable:
+}
+
+void bar(const Printy& a, const Printy& b);
+
+// CHECK: define dso_local ptr @_Z5test2v()
+coroutine test2() {
+ // CHECK-NOT: @_ZN6PrintyD1Ev
+ bar(
+ Printy(),
+ co_await Awaiter{}
+ // CHECK: await.cleanup:
+ // CHECK-NEXT: br label %cleanup{{.*}}.from.await.cleanup
+ // CHECK-NOT: @_ZN6PrintyD1Ev
+ );
+ // CHECK: await.ready:
+ // CHECK: call void @_ZN6PrintyD1Ev
+ // CHECK-NOT: @_ZN6PrintyD1Ev
+
+ // CHECK: cleanup{{.*}}:
+ // CHECK: call void @_ZN6PrintyD1Ev
+ // CHECK-NOT: @_ZN6PrintyD1Ev
+
+ // CHECK: unreachable:
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/80698
More information about the cfe-commits
mailing list