[clang] [codegen] Emit cleanups for branch in stmt-expr and coro suspensions [take-2] (PR #85398)
Utkarsh Saxena via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 15 08:22:19 PDT 2024
https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/85398
>From 0654f624b3b60026398e8b0168623a85e3129630 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Fri, 15 Mar 2024 13:19:36 +0000
Subject: [PATCH 1/2] [codegen] Emit cleanups for branch in stmt-expr and coro
suspensions
---
clang/lib/CodeGen/CGCleanup.cpp | 7 +++++--
clang/lib/CodeGen/CGExprAgg.cpp | 13 +++++++------
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index f87caf050eeaa7..c22d4da0fefc3c 100644
--- a/clang/lib/CodeGen/CGCleanup.cpp
+++ b/clang/lib/CodeGen/CGCleanup.cpp
@@ -694,7 +694,8 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
// - whether there's a fallthrough
llvm::BasicBlock *FallthroughSource = Builder.GetInsertBlock();
- bool HasFallthrough = (FallthroughSource != nullptr && IsActive);
+ bool HasFallthrough =
+ FallthroughSource != nullptr && (IsActive || HasExistingBranches);
// Branch-through fall-throughs leave the insertion point set to the
// end of the last cleanup, which points to the current scope. The
@@ -719,7 +720,9 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
// If we have a prebranched fallthrough into an inactive normal
// cleanup, rewrite it so that it leads to the appropriate place.
- if (Scope.isNormalCleanup() && HasPrebranchedFallthrough && !IsActive) {
+ if (Scope.isNormalCleanup() && HasPrebranchedFallthrough &&
+ !RequiresNormalCleanup) {
+ assert(!IsActive);
llvm::BasicBlock *prebranchDest;
// If the prebranch is semantically branching through the next
diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 5190b22bcc1622..7e62599089b8df 100644
--- a/clang/lib/CodeGen/CGExprAgg.cpp
+++ b/clang/lib/CodeGen/CGExprAgg.cpp
@@ -15,6 +15,7 @@
#include "CodeGenFunction.h"
#include "CodeGenModule.h"
#include "ConstantEmitter.h"
+#include "EHScopeStack.h"
#include "TargetInfo.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Attr.h"
@@ -1389,15 +1390,15 @@ AggExprEmitter::VisitLambdaExpr(LambdaExpr *E) {
if (QualType::DestructionKind DtorKind =
CurField->getType().isDestructedType()) {
assert(LV.isSimple());
- if (CGF.needsEHCleanup(DtorKind)) {
+ if (DtorKind) {
if (!CleanupDominator)
CleanupDominator = CGF.Builder.CreateAlignedLoad(
CGF.Int8Ty,
llvm::Constant::getNullValue(CGF.Int8PtrTy),
CharUnits::One()); // placeholder
- CGF.pushDestroy(EHCleanup, LV.getAddress(CGF), CurField->getType(),
- CGF.getDestroyer(DtorKind), false);
+ CGF.pushDestroy(NormalAndEHCleanup, LV.getAddress(CGF),
+ CurField->getType(), CGF.getDestroyer(DtorKind), false);
Cleanups.push_back(CGF.EHStack.stable_begin());
}
}
@@ -1806,9 +1807,9 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr(
if (QualType::DestructionKind dtorKind
= field->getType().isDestructedType()) {
assert(LV.isSimple());
- if (CGF.needsEHCleanup(dtorKind)) {
- CGF.pushDestroy(EHCleanup, LV.getAddress(CGF), field->getType(),
- CGF.getDestroyer(dtorKind), false);
+ if (dtorKind) {
+ CGF.pushDestroy(NormalAndEHCleanup, LV.getAddress(CGF),
+ field->getType(), CGF.getDestroyer(dtorKind), false);
addCleanup(CGF.EHStack.stable_begin());
pushedCleanup = true;
}
>From 1b7c160b030f3a765d40723bbbf9a7daa4a1e090 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Fri, 15 Mar 2024 15:22:07 +0000
Subject: [PATCH 2/2] add tests
---
.../control-flow-in-expr-cleanup.cpp | 172 ++++++++++++++++++
1 file changed, 172 insertions(+)
create mode 100644 clang/test/CodeGenCoroutines/control-flow-in-expr-cleanup.cpp
diff --git a/clang/test/CodeGenCoroutines/control-flow-in-expr-cleanup.cpp b/clang/test/CodeGenCoroutines/control-flow-in-expr-cleanup.cpp
new file mode 100644
index 00000000000000..d703532b5a10b9
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/control-flow-in-expr-cleanup.cpp
@@ -0,0 +1,172 @@
+// RUN: %clang_cc1 --std=c++20 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct Printy {
+ Printy(const char *name) : name(name) {}
+ ~Printy() {}
+ const char *name;
+};
+
+struct coroutine {
+ struct promise_type;
+ std::coroutine_handle<promise_type> handle;
+ ~coroutine() {
+ if (handle) handle.destroy();
+ }
+};
+
+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 Awaiter : std::suspend_always {
+ Printy await_resume() { return {"awaited"}; }
+};
+
+int foo() { return 2; }
+
+struct PrintiesCopy {
+ Printy a;
+ Printy b;
+ Printy c;
+};
+
+void ParenInit() {
+ // CHECK: define dso_local void @_Z9ParenInitv()
+ // CHECK: [[CLEANUP_DEST:%.+]] = alloca i32, align 4
+ PrintiesCopy ps(Printy("a"),
+ // CHECK: call void @_ZN6PrintyC1EPKc
+ ({
+ if (foo()) return;
+ // CHECK: if.then:
+ // CHECK-NEXT: store i32 1, ptr [[CLEANUP_DEST]], align 4
+ // CHECK-NEXT: br label %cleanup
+ Printy("b");
+ // CHECK: if.end:
+ // CHECK-NEXT: call void @_ZN6PrintyC1EPKc
+ }),
+ ({
+ if (foo()) return;
+ // CHECK: if.then{{.*}}:
+ // CHECK-NEXT: store i32 1, ptr [[CLEANUP_DEST]], align 4
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label %cleanup
+ Printy("c");
+ // CHECK: if.end{{.*}}:
+ // CHECK-NEXT: call void @_ZN6PrintyC1EPKc
+ // CHECK-NEXT: call void @_ZN12PrintiesCopyD1Ev
+ // CHECK-NEXT: br label %return
+ }));
+ // CHECK: cleanup:
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label %return
+}
+
+coroutine ParenInitCoro() {
+ // CHECK: define dso_local void @_Z13ParenInitCorov
+ // CHECK: [[ACTIVE1:%.+]] = alloca i1, align 1
+ // CHECK: [[ACTIVE2:%.+]] = alloca i1, align 1
+ PrintiesCopy ps(Printy("a"), Printy("b"),
+ // CHECK: call void @_ZN6PrintyC1EPKc
+ // CHECK-NEXT: store i1 true, ptr [[ACTIVE1]].reload.addr, align 1
+ // CHECK-NEXT: store i1 true, ptr [[ACTIVE2]].reload.addr, align 1
+ // CHECK: call void @_ZN6PrintyC1EPKc
+ co_await Awaiter{}
+
+ // CHECK: await.cleanup:
+ // CHECK-NEXT: br label %[[CLEANUP:.+]].from.await.cleanup
+
+ // CHECK: [[CLEANUP]].from.await.cleanup:
+ // CHECK: br label %[[CLEANUP]]
+
+ // CHECK: await.ready:
+ // CHECK: store i1 false, ptr [[ACTIVE1]].reload.addr, align 1
+ // CHECK-NEXT: store i1 false, ptr [[ACTIVE2]].reload.addr, align 1
+ // CHECK-NEXT: br label %[[CLEANUP]].from.await.ready
+
+ // CHECK: [[CLEANUP]].from.await.ready:
+ // CHECK: br label %[[CLEANUP]]
+
+ // CHECK: [[CLEANUP]]:
+ // CHECK: [[IS_ACTIVE1:%.+]] = load i1, ptr [[ACTIVE1]].reload.addr, align 1
+ // CHECK-NEXT: br i1 [[IS_ACTIVE1]], label %[[ACTION1:.+]], label %[[DONE1:.+]]
+
+ // CHECK: [[ACTION1]]:
+ // CHECK: call void @_ZN6PrintyD1Ev
+ // CHECK: br label %[[DONE1]]
+
+ // CHECK: [[DONE1]]:
+ // CHECK: [[IS_ACTIVE2:%.+]] = load i1, ptr [[ACTIVE2]].reload.addr, align 1
+ // CHECK-NEXT: br i1 [[IS_ACTIVE2]], label %[[ACTION2:.+]], label %[[DONE2:.+]]
+
+ // CHECK: [[ACTION2]]:
+ // CHECK: call void @_ZN6PrintyD1Ev
+ // CHECK: br label %[[DONE2]]
+ );
+}
+
+// TODO: Add more assertions after preliminary review.
+// struct S {
+// Printy arr1[2];
+// Printy arr2[2];
+// Printy p;
+// };
+
+// void ArraySubobjects() {
+// S s{{Printy("a"), Printy("b")},
+// {Printy("a"), ({
+// if (foo() == 1) {
+// return;
+// }
+// Printy("b");
+// })},
+// ({
+// if (foo() == 2) {
+// return;
+// }
+// Printy("b");
+// })};
+// }
+
+// coroutine ArraySubobjectsCoro() {
+// S s{{Printy("a"), Printy("b")},
+// {Printy("a"), co_await Awaiter{}},
+// co_await Awaiter{}};
+// }
+
+// struct A {
+// Printy a;
+// };
+// struct B : A {
+// Printy b;
+// Printy c;
+// };
+
+// void BaseClassCtors() {
+// auto S = B({Printy("a")}, Printy("b"), ({
+// return;
+// Printy("c");
+// }));
+// }
+
+// coroutine BaseClassCtorsCoro() {
+// auto S = B({Printy("a")}, Printy("b"), co_await Awaiter{});
+// }
+
+// void LambdaInit() {
+// auto S = [a = Printy("a"), b = ({
+// return;
+// Printy("b");
+// })]() { return a; };
+// }
+
+// coroutine LambdaInitCoro() {
+// auto S = [a = Printy("a"), b = co_await Awaiter{}]() { return a; };
+// }
More information about the cfe-commits
mailing list