[clang] [clang-tools-extra] [codegen] Emit cleanups for lifetime-extended temporaries when an expr contains control-flow (PR #80698)
Utkarsh Saxena via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 9 12:18:19 PST 2024
https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/80698
>From 28d90711ff8e4924135d4bd4e5f252d96ac41b93 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 23 Jan 2024 20:18:25 +0000
Subject: [PATCH 1/9] [misc-coroutine-hostile-raii] Use getOperand instead of
getCommonExpr
---
.../misc/CoroutineHostileRAIICheck.cpp | 2 +-
.../checkers/misc/coroutine-hostile-raii.cpp | 34 ++++++++++++++++++-
2 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp
index a0e8700b0522bc..360335b86c6418 100644
--- a/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/CoroutineHostileRAIICheck.cpp
@@ -56,7 +56,7 @@ AST_MATCHER_P(Stmt, forEachPrevStmt, ast_matchers::internal::Matcher<Stmt>,
// Matches the expression awaited by the `co_await`.
AST_MATCHER_P(CoawaitExpr, awaitable, ast_matchers::internal::Matcher<Expr>,
InnerMatcher) {
- if (Expr *E = Node.getCommonExpr())
+ if (Expr *E = Node.getOperand())
return InnerMatcher.matches(*E, Finder, Builder);
return false;
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/coroutine-hostile-raii.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/coroutine-hostile-raii.cpp
index 55a7e4b8f2954a..c23c355dac1b2a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/coroutine-hostile-raii.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/coroutine-hostile-raii.cpp
@@ -1,7 +1,7 @@
// RUN: %check_clang_tidy -std=c++20 %s misc-coroutine-hostile-raii %t \
// RUN: -config="{CheckOptions: {\
// RUN: misc-coroutine-hostile-raii.RAIITypesList: 'my::Mutex; ::my::other::Mutex', \
-// RUN: misc-coroutine-hostile-raii.AllowedAwaitablesList: 'safe::awaitable; ::my::other::awaitable' \
+// RUN: misc-coroutine-hostile-raii.AllowedAwaitablesList: 'safe::awaitable; ::transformable::awaitable' \
// RUN: }}"
namespace std {
@@ -136,6 +136,9 @@ ReturnObject scopedLockableTest() {
absl::Mutex no_warning_5;
}
+// ================================================================================
+// Safe awaitable
+// ================================================================================
namespace safe {
struct awaitable {
bool await_ready() noexcept { return false; }
@@ -150,6 +153,32 @@ ReturnObject RAIISafeSuspendTest() {
co_await other{};
}
+// ================================================================================
+// Safe transformable awaitable
+// ================================================================================
+struct transformable { struct awaitable{}; };
+using alias_transformable_awaitable = transformable::awaitable;
+struct UseTransformAwaitable {
+ struct promise_type {
+ UseTransformAwaitable get_return_object() { return {}; }
+ std::suspend_always initial_suspend() { return {}; }
+ std::suspend_always final_suspend() noexcept { return {}; }
+ void unhandled_exception() {}
+ std::suspend_always await_transform(transformable::awaitable) { return {}; }
+ };
+};
+
+auto retAwaitable() { return transformable::awaitable{}; }
+UseTransformAwaitable RAIISafeSuspendTest2() {
+ absl::Mutex a;
+ co_await retAwaitable();
+ co_await transformable::awaitable{};
+ co_await alias_transformable_awaitable{};
+}
+
+// ================================================================================
+// Lambdas
+// ================================================================================
void lambda() {
absl::Mutex no_warning;
auto lambda = []() -> ReturnObject {
@@ -164,6 +193,9 @@ void lambda() {
absl::Mutex no_warning_2;
}
+// ================================================================================
+// Denylisted RAII
+// ================================================================================
template<class T>
ReturnObject raii_in_template(){
T a;
>From 442809ce6047f8f83c9f3f54b17182c18535bb03 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Mon, 5 Feb 2024 15:52:56 +0000
Subject: [PATCH 2/9] [codegen] Emit cleanups for lifetime-extended temporaries
when statment-expression has control-flow
---
clang/lib/CodeGen/CGCleanup.cpp | 31 +++++--
clang/lib/CodeGen/CodeGenFunction.h | 4 +
.../return-in-stmt-expr-cleanup.cpp | 37 +++++++++
.../coro-suspend-in-agg-init.cpp | 82 +++++++++++++++++++
4 files changed, 145 insertions(+), 9 deletions(-)
create mode 100644 clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp
create mode 100644 clang/test/CodeGenCoroutines/coro-suspend-in-agg-init.cpp
diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index f87caf050eeaa7..da0528b271aa37 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 143ad64e8816b1..ac55e84034bc60 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 00000000000000..214becd81e61af
--- /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 00000000000000..362e212b7fba39
--- /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:
+}
>From 41258086773829db7f17afa9df192d6ca56b2bfa Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Mon, 5 Feb 2024 16:52:25 +0000
Subject: [PATCH 3/9] format
---
clang/lib/CodeGen/CGCleanup.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index da0528b271aa37..39c65c0b07c6f6 100644
--- a/clang/lib/CodeGen/CGCleanup.cpp
+++ b/clang/lib/CodeGen/CGCleanup.cpp
@@ -489,7 +489,8 @@ void CodeGenFunction::PopCleanupBlocks(
}
/// Adds deferred lifetime-extended cleanups onto the EH stack.
-void CodeGenFunction::AddLifetimeExtendedCleanups(size_t OldLifetimeExtendedSize) {
+void CodeGenFunction::AddLifetimeExtendedCleanups(
+ size_t OldLifetimeExtendedSize) {
for (size_t I = OldLifetimeExtendedSize,
E = LifetimeExtendedCleanupStack.size();
I != E;) {
>From 35437116444d9ac2827634456a449c19394434d7 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 6 Feb 2024 14:27:40 +0000
Subject: [PATCH 4/9] Emit not-all but only necessary lifetime-extended
cleanups for each branch
---
clang/lib/CodeGen/CGCleanup.cpp | 3 +-
clang/lib/CodeGen/CGStmt.cpp | 2 +-
clang/lib/CodeGen/CodeGenFunction.h | 17 ++++-
.../control-flow-in-stmt-expr-cleanup.cpp | 68 +++++++++++++++++++
.../return-in-stmt-expr-cleanup.cpp | 37 ----------
5 files changed, 85 insertions(+), 42 deletions(-)
create mode 100644 clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp
delete mode 100644 clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp
diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index 39c65c0b07c6f6..a5a49917d108f9 100644
--- a/clang/lib/CodeGen/CGCleanup.cpp
+++ b/clang/lib/CodeGen/CGCleanup.cpp
@@ -1114,7 +1114,8 @@ void CodeGenFunction::EmitBranchThroughCleanup(JumpDest Dest) {
// EHStack. Do not remove them from lifetime-extended stack, they need to be
// emitted again after the expression completes.
RunCleanupsScope LifetimeExtendedCleanups(*this);
- AddLifetimeExtendedCleanups(0);
+ if(Dest.isValid())
+ AddLifetimeExtendedCleanups(Dest.getLifetimeExtendedDepth());
// Create the branch.
llvm::BranchInst *BI = Builder.CreateBr(Dest.getBlock());
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index beff0ad9da2709..8d7e2616c168c0 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -628,7 +628,7 @@ CodeGenFunction::getJumpDestForLabel(const LabelDecl *D) {
// Create, but don't insert, the new block.
Dest = JumpDest(createBasicBlock(D->getName()),
- EHScopeStack::stable_iterator::invalid(),
+ EHScopeStack::stable_iterator::invalid(), 0,
NextCleanupDestIndex++);
return Dest;
}
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index ac55e84034bc60..78bc45dca2936c 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -238,15 +238,20 @@ class CodeGenFunction : public CodeGenTypeCache {
/// A jump destination is an abstract label, branching to which may
/// require a jump out through normal cleanups.
struct JumpDest {
- JumpDest() : Block(nullptr), Index(0) {}
+ JumpDest() : Block(nullptr), LifetimeExtendedDepth(0), Index(0) {}
JumpDest(llvm::BasicBlock *Block, EHScopeStack::stable_iterator Depth,
- unsigned Index)
- : Block(Block), ScopeDepth(Depth), Index(Index) {}
+ unsigned LifetimeExtendedScopeDepth, unsigned Index)
+ : Block(Block), ScopeDepth(Depth),
+ LifetimeExtendedDepth(LifetimeExtendedScopeDepth), Index(Index) {
+ }
bool isValid() const { return Block != nullptr; }
llvm::BasicBlock *getBlock() const { return Block; }
EHScopeStack::stable_iterator getScopeDepth() const { return ScopeDepth; }
unsigned getDestIndex() const { return Index; }
+ unsigned getLifetimeExtendedDepth() const {
+ return LifetimeExtendedDepth;
+ }
// This should be used cautiously.
void setScopeDepth(EHScopeStack::stable_iterator depth) {
@@ -256,6 +261,11 @@ class CodeGenFunction : public CodeGenTypeCache {
private:
llvm::BasicBlock *Block;
EHScopeStack::stable_iterator ScopeDepth;
+
+ // Size of the lifetime-extended cleanup stack in destination scope.
+ // This can only occur when nested stmt-expr's contains branches.
+ // This is useful to emit only the necessary lifeitme-extended cleanups.
+ unsigned LifetimeExtendedDepth;
unsigned Index;
};
@@ -1163,6 +1173,7 @@ class CodeGenFunction : public CodeGenTypeCache {
JumpDest getJumpDestInCurrentScope(llvm::BasicBlock *Target) {
return JumpDest(Target,
EHStack.getInnermostNormalCleanup(),
+ LifetimeExtendedCleanupStack.size(),
NextCleanupDestIndex++);
}
diff --git a/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp b/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp
new file mode 100644
index 00000000000000..a53b0fcf040927
--- /dev/null
+++ b/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp
@@ -0,0 +1,68 @@
+// 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() {
+// CHECK: define dso_local void @_Z3barv()
+// CHECK-NOT: call void @_ZN6PrintyD1Ev
+ Printies p2{
+ Printy(),
+ ({
+ if(foo()) {
+ // CHECK: if.then:
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label %return
+ // CHECK-NOT: call void @_ZN6PrintyD1Ev
+ return;
+ }
+ Printy();
+ })};
+ // CHECK: if.end:
+ // CHECK: call void @_ZN8PrintiesD1Ev
+ // CHECK: call void @_ZN6PrintyD1Ev
+ // CHECK: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label %return
+ return;
+ // CHECK-NOT: call void @_ZN6PrintyD1Ev
+}
+
+
+void test_break() {
+// CHECK: define dso_local void @_Z10test_breakv()
+// CHECK-NOT: call void @_ZN6PrintyD1Ev
+ Printies p2{Printy(), ({
+ for (;;) {
+ Printies p3{Printy(), ({
+ if(foo()) {
+ break;
+ // CHECK: if.then:
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label %for.end
+ // CHECK-NOT: call void @_ZN6PrintyD1Ev
+ }
+ Printy();
+ })};
+ // CHECK: if.end:
+ // CHECK: call void @_ZN6PrintyD1Ev
+ // CHECK: call void @_ZN6PrintyD1Ev
+ // CHECK-NOT: call void @_ZN6PrintyD1Ev
+ }
+ Printy();
+ })};
+ // CHECK: for.end:
+ // CHECK-COUNT-2: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: ret void
+ // CHECK-NOT: call void @_ZN6PrintyD1Ev
+}
+
diff --git a/clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp b/clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp
deleted file mode 100644
index 214becd81e61af..00000000000000
--- a/clang/test/CodeGenCXX/return-in-stmt-expr-cleanup.cpp
+++ /dev/null
@@ -1,37 +0,0 @@
-// 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;
-}
-
>From d23cdb3fb5998cab7e7102ac715eca8e0bfe57c0 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 6 Feb 2024 14:34:25 +0000
Subject: [PATCH 5/9] format
---
clang/lib/CodeGen/CGCleanup.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index a5a49917d108f9..e97441310e34c6 100644
--- a/clang/lib/CodeGen/CGCleanup.cpp
+++ b/clang/lib/CodeGen/CGCleanup.cpp
@@ -1114,7 +1114,7 @@ void CodeGenFunction::EmitBranchThroughCleanup(JumpDest Dest) {
// EHStack. Do not remove them from lifetime-extended stack, they need to be
// emitted again after the expression completes.
RunCleanupsScope LifetimeExtendedCleanups(*this);
- if(Dest.isValid())
+ if (Dest.isValid())
AddLifetimeExtendedCleanups(Dest.getLifetimeExtendedDepth());
// Create the branch.
>From dc3659a4f061ab05fd51ed39d673c5a2710cd41f Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 6 Feb 2024 14:35:24 +0000
Subject: [PATCH 6/9] format
---
clang/lib/CodeGen/CodeGenFunction.h | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 78bc45dca2936c..11e4c6481776f7 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -249,9 +249,7 @@ class CodeGenFunction : public CodeGenTypeCache {
llvm::BasicBlock *getBlock() const { return Block; }
EHScopeStack::stable_iterator getScopeDepth() const { return ScopeDepth; }
unsigned getDestIndex() const { return Index; }
- unsigned getLifetimeExtendedDepth() const {
- return LifetimeExtendedDepth;
- }
+ unsigned getLifetimeExtendedDepth() const { return LifetimeExtendedDepth; }
// This should be used cautiously.
void setScopeDepth(EHScopeStack::stable_iterator depth) {
@@ -1171,8 +1169,7 @@ class CodeGenFunction : public CodeGenTypeCache {
/// target of a potentially scope-crossing jump; get a stable handle
/// to which we can perform this jump later.
JumpDest getJumpDestInCurrentScope(llvm::BasicBlock *Target) {
- return JumpDest(Target,
- EHStack.getInnermostNormalCleanup(),
+ return JumpDest(Target, EHStack.getInnermostNormalCleanup(),
LifetimeExtendedCleanupStack.size(),
NextCleanupDestIndex++);
}
>From ddf7c244e6d142cf61c7277d06ead2e0fb79f4a3 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Tue, 6 Feb 2024 14:52:44 +0000
Subject: [PATCH 7/9] format
---
clang/lib/CodeGen/CodeGenFunction.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 11e4c6481776f7..ec9f0de0c03385 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -242,8 +242,7 @@ class CodeGenFunction : public CodeGenTypeCache {
JumpDest(llvm::BasicBlock *Block, EHScopeStack::stable_iterator Depth,
unsigned LifetimeExtendedScopeDepth, unsigned Index)
: Block(Block), ScopeDepth(Depth),
- LifetimeExtendedDepth(LifetimeExtendedScopeDepth), Index(Index) {
- }
+ LifetimeExtendedDepth(LifetimeExtendedScopeDepth), Index(Index) {}
bool isValid() const { return Block != nullptr; }
llvm::BasicBlock *getBlock() const { return Block; }
>From a58284b872ca4118426d0e9339118c025ef8a0d3 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Fri, 9 Feb 2024 20:03:27 +0000
Subject: [PATCH 8/9] Add BranchInExpr cleanups kind to handle more missing
dtor calls
---
clang/lib/CodeGen/CGCleanup.cpp | 40 +++---
clang/lib/CodeGen/CGDecl.cpp | 15 ++-
clang/lib/CodeGen/CGExprAgg.cpp | 19 ++-
clang/lib/CodeGen/CGStmt.cpp | 6 +-
clang/lib/CodeGen/CodeGenFunction.cpp | 3 +
clang/lib/CodeGen/CodeGenFunction.h | 117 ++++++++++++-----
clang/lib/CodeGen/EHScopeStack.h | 10 ++
.../control-flow-in-stmt-expr-cleanup.cpp | 119 +++++++++++++++++-
8 files changed, 260 insertions(+), 69 deletions(-)
diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index e97441310e34c6..af787440f8847b 100644
--- a/clang/lib/CodeGen/CGCleanup.cpp
+++ b/clang/lib/CodeGen/CGCleanup.cpp
@@ -18,6 +18,8 @@
#include "CGCleanup.h"
#include "CodeGenFunction.h"
+#include "EHScopeStack.h"
+#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/SaveAndRestore.h"
using namespace clang;
@@ -488,29 +490,22 @@ void CodeGenFunction::PopCleanupBlocks(
}
}
-/// 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;) {
+void CodeGenFunction::AddDeferredCleanups(DeferredCleanupStack &Cleanups,
+ size_t OldSize) {
+ for (size_t I = OldSize, E = Cleanups.size(); I != E;) {
// Alignment should be guaranteed by the vptrs in the individual cleanups.
- assert((I % alignof(LifetimeExtendedCleanupHeader) == 0) &&
+ assert((I % alignof(DeferredCleanupHeader) == 0) &&
"misaligned cleanup stack entry");
- LifetimeExtendedCleanupHeader &Header =
- reinterpret_cast<LifetimeExtendedCleanupHeader&>(
- LifetimeExtendedCleanupStack[I]);
+ DeferredCleanupHeader &Header =
+ reinterpret_cast<DeferredCleanupHeader &>(Cleanups[I]);
I += sizeof(Header);
- EHStack.pushCopyOfCleanup(Header.getKind(),
- &LifetimeExtendedCleanupStack[I],
- Header.getSize());
+ EHStack.pushCopyOfCleanup(Header.getKind(), &Cleanups[I], Header.getSize());
I += Header.getSize();
if (Header.isConditional()) {
- Address ActiveFlag =
- reinterpret_cast<Address &>(LifetimeExtendedCleanupStack[I]);
+ Address ActiveFlag = reinterpret_cast<Address &>(Cleanups[I]);
initFullExprCleanupWithFlag(ActiveFlag);
I += sizeof(ActiveFlag);
}
@@ -524,8 +519,8 @@ void CodeGenFunction::PopCleanupBlocks(
std::initializer_list<llvm::Value **> ValuesToReload) {
PopCleanupBlocks(Old, ValuesToReload);
- // Move our deferred cleanups onto the EH stack.
- AddLifetimeExtendedCleanups(OldLifetimeExtendedSize);
+ // Move our deferred lifetime-extended cleanups onto the EH stack.
+ AddDeferredCleanups(LifetimeExtendedCleanupStack, OldLifetimeExtendedSize);
LifetimeExtendedCleanupStack.resize(OldLifetimeExtendedSize);
}
@@ -1109,13 +1104,12 @@ 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);
+ // If we are emitting a branch in a partial expression, add deferred cleanups
+ // to EHStack, which would otherwise have only been emitted after the full
+ // expression.
+ RunCleanupsScope BranchInExprCleanups(*this);
if (Dest.isValid())
- AddLifetimeExtendedCleanups(Dest.getLifetimeExtendedDepth());
+ AddDeferredCleanups(BranchInExprCleanupStack, Dest.getBranchInExprDepth());
// Create the branch.
llvm::BranchInst *BI = Builder.CreateBr(Dest.getBlock());
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index bbe14ef4c17244..66df9186155d9c 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//
+#include "Address.h"
#include "CGBlocks.h"
#include "CGCXXABI.h"
#include "CGCleanup.h"
@@ -19,6 +20,7 @@
#include "CodeGenFunction.h"
#include "CodeGenModule.h"
#include "ConstantEmitter.h"
+#include "EHScopeStack.h"
#include "PatternInit.h"
#include "TargetInfo.h"
#include "clang/AST/ASTContext.h"
@@ -2209,8 +2211,11 @@ void CodeGenFunction::pushLifetimeExtendedDestroy(CleanupKind cleanupKind,
static_cast<CleanupKind>(cleanupKind & ~NormalCleanup), addr, type,
destroyer, useEHCleanupForArray);
- return pushCleanupAfterFullExprWithActiveFlag<DestroyObject>(
- cleanupKind, Address::invalid(), addr, type, destroyer, useEHCleanupForArray);
+ pushDestroy(BranchInExprCleanup, addr, type, destroyer,
+ useEHCleanupForArray);
+ return pushDeferredCleanup<DestroyObject>(
+ LifetimeExtendedCleanupStack, cleanupKind, Address::invalid(), addr,
+ type, destroyer, useEHCleanupForArray);
}
// Otherwise, we should only destroy the object if it's been initialized.
@@ -2232,9 +2237,9 @@ void CodeGenFunction::pushLifetimeExtendedDestroy(CleanupKind cleanupKind,
initFullExprCleanupWithFlag(ActiveFlag);
}
- pushCleanupAfterFullExprWithActiveFlag<ConditionalCleanupType>(
- cleanupKind, ActiveFlag, SavedAddr, type, destroyer,
- useEHCleanupForArray);
+ pushDeferredCleanup<ConditionalCleanupType>(
+ LifetimeExtendedCleanupStack, cleanupKind, ActiveFlag, SavedAddr, type,
+ destroyer, useEHCleanupForArray);
}
/// emitDestroy - Immediately perform the destruction of the given
diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp
index 22f55fe9aac904..bd2e9bc0f82a91 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"
@@ -553,6 +554,7 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType,
Address endOfInit = Address::invalid();
EHScopeStack::stable_iterator cleanup;
llvm::Instruction *cleanupDominator = nullptr;
+ auto myDtorKind = dtorKind;
if (CGF.needsEHCleanup(dtorKind)) {
// In principle we could tell the cleanup where we are more
// directly, but the control flow can get so varied here that it
@@ -580,6 +582,7 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType,
// elements have been initialized.
llvm::Value *element = begin;
+ CodeGenFunction::RestoreBranchInExpr branchInExpr(CGF);
// Emit the explicit initializers.
for (uint64_t i = 0; i != NumInitElements; ++i) {
// Advance to the next element.
@@ -592,10 +595,14 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType,
// observed to be unnecessary.
if (endOfInit.isValid()) Builder.CreateStore(element, endOfInit);
}
-
- LValue elementLV = CGF.MakeAddrLValue(
- Address(element, llvmElementType, elementAlign), elementType);
+ Address address = Address(element, llvmElementType, elementAlign);
+ LValue elementLV = CGF.MakeAddrLValue(address, elementType);
EmitInitializationToLValue(Args[i], elementLV);
+ // Schedule to emit element cleanup if we see a branch in the array
+ // initialisation statement.
+ if (CGF.needsBranchCleanup(myDtorKind))
+ CGF.pushDestroy(BranchInExprCleanup, address, elementType,
+ CGF.getDestroyer(myDtorKind), false /*oder true ?*/);
}
// Check whether there's a non-trivial array-fill expression.
@@ -1754,7 +1761,7 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr(
return;
}
-
+ CodeGenFunction::RestoreBranchInExpr branchInExpr(CGF);
// Here we iterate over the fields; this makes it simpler to both
// default-initialize fields and skip over unnamed fields.
for (const auto *field : record->fields()) {
@@ -1793,6 +1800,10 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr(
if (QualType::DestructionKind dtorKind
= field->getType().isDestructedType()) {
assert(LV.isSimple());
+ if(CGF.needsBranchCleanup(dtorKind)) {
+ CGF.pushDestroy(BranchInExprCleanup, LV.getAddress(CGF),
+ field->getType(), CGF.getDestroyer(dtorKind), false);
+ }
if (CGF.needsEHCleanup(dtorKind)) {
CGF.pushDestroy(EHCleanup, LV.getAddress(CGF), field->getType(),
CGF.getDestroyer(dtorKind), false);
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 8d7e2616c168c0..9f1ab1e7817179 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -627,9 +627,9 @@ CodeGenFunction::getJumpDestForLabel(const LabelDecl *D) {
if (Dest.isValid()) return Dest;
// Create, but don't insert, the new block.
- Dest = JumpDest(createBasicBlock(D->getName()),
- EHScopeStack::stable_iterator::invalid(), 0,
- NextCleanupDestIndex++);
+ Dest = JumpDest(
+ createBasicBlock(D->getName()), EHScopeStack::stable_iterator::invalid(),
+ EHScopeStack::stable_iterator::invalid(), 0, NextCleanupDestIndex++);
return Dest;
}
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 1ad905078d349c..7bff6c89de563c 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -331,6 +331,9 @@ static void EmitIfUsed(CodeGenFunction &CGF, llvm::BasicBlock *BB) {
void CodeGenFunction::FinishFunction(SourceLocation EndLoc) {
assert(BreakContinueStack.empty() &&
"mismatched push/pop in break/continue stack!");
+ assert(LifetimeExtendedCleanupStack.empty() &&
+ "mismatched push/pop in stack!");
+ assert(BranchInExprCleanupStack.empty() && "mismatched push/pop in stack!");
bool OnlySimpleReturnStmts = NumSimpleReturnExprs > 0
&& NumSimpleReturnExprs == NumReturnExprs
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index ec9f0de0c03385..5310ec8440fd5a 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -238,17 +238,19 @@ class CodeGenFunction : public CodeGenTypeCache {
/// A jump destination is an abstract label, branching to which may
/// require a jump out through normal cleanups.
struct JumpDest {
- JumpDest() : Block(nullptr), LifetimeExtendedDepth(0), Index(0) {}
+ JumpDest() : Block(nullptr), BranchInExprDepth(0), Index(0) {}
JumpDest(llvm::BasicBlock *Block, EHScopeStack::stable_iterator Depth,
- unsigned LifetimeExtendedScopeDepth, unsigned Index)
- : Block(Block), ScopeDepth(Depth),
- LifetimeExtendedDepth(LifetimeExtendedScopeDepth), Index(Index) {}
+ EHScopeStack::stable_iterator EHScopeDepth,
+ unsigned BranchInExprDepth, unsigned Index)
+ : Block(Block), ScopeDepth(Depth), EHScopeDepth(EHScopeDepth),
+ BranchInExprDepth(BranchInExprDepth), Index(Index) {}
bool isValid() const { return Block != nullptr; }
llvm::BasicBlock *getBlock() const { return Block; }
EHScopeStack::stable_iterator getScopeDepth() const { return ScopeDepth; }
+ EHScopeStack::stable_iterator getEHScopeDepth() const { return EHScopeDepth; }
unsigned getDestIndex() const { return Index; }
- unsigned getLifetimeExtendedDepth() const { return LifetimeExtendedDepth; }
+ unsigned getBranchInExprDepth() const { return BranchInExprDepth; }
// This should be used cautiously.
void setScopeDepth(EHScopeStack::stable_iterator depth) {
@@ -258,11 +260,12 @@ class CodeGenFunction : public CodeGenTypeCache {
private:
llvm::BasicBlock *Block;
EHScopeStack::stable_iterator ScopeDepth;
+ EHScopeStack::stable_iterator EHScopeDepth;
- // Size of the lifetime-extended cleanup stack in destination scope.
- // This can only occur when nested stmt-expr's contains branches.
- // This is useful to emit only the necessary lifeitme-extended cleanups.
- unsigned LifetimeExtendedDepth;
+ // Size of the branch-in-expr cleanup stack in destination scope.
+ // All cleanups beyond this depth would be emitted on encountering a branch
+ // to this destination inside an expression.
+ unsigned BranchInExprDepth;
unsigned Index;
};
@@ -633,7 +636,33 @@ class CodeGenFunction : public CodeGenTypeCache {
llvm::DenseMap<const VarDecl *, llvm::Value *> NRVOFlags;
EHScopeStack EHStack;
- llvm::SmallVector<char, 256> LifetimeExtendedCleanupStack;
+
+ using DeferredCleanupStack = llvm::SmallVector<char, 256>;
+
+ // Deferred cleanups for lifetime-extended temporaries. Such cleanups are
+ // deferred until the end of the full expression, after which these are added
+ // to the EHStack.
+ DeferredCleanupStack LifetimeExtendedCleanupStack;
+
+ // Branch-in-expression cleanups include the cleanups which are not yet added
+ // to the EHStack while building an expression.
+ // Cleanups from this stack are only emitted when encountering a branch while
+ // building an expression (eg: branches in stmt-expr or coroutine
+ // suspensions). Otherwise, these should be cleared the end of the expression and
+ // added separately to the EHStack.
+ DeferredCleanupStack BranchInExprCleanupStack;
+
+ class RestoreBranchInExpr {
+ public:
+ RestoreBranchInExpr(CodeGenFunction &CGF)
+ : CGF(CGF), OldSize(CGF.BranchInExprCleanupStack.size()) {}
+ ~RestoreBranchInExpr() { CGF.BranchInExprCleanupStack.resize(OldSize); }
+
+ private:
+ CodeGenFunction &CGF;
+ size_t OldSize;
+ };
+
llvm::SmallVector<const JumpDest *, 2> SEHTryEpilogueStack;
llvm::Instruction *CurrentFuncletPad = nullptr;
@@ -653,8 +682,8 @@ class CodeGenFunction : public CodeGenTypeCache {
}
};
- /// Header for data within LifetimeExtendedCleanupStack.
- struct LifetimeExtendedCleanupHeader {
+ /// Header for data within deferred cleanup stacks.
+ struct DeferredCleanupHeader {
/// The size of the following cleanup object.
unsigned Size;
/// The kind of cleanup to push: a value from the CleanupKind enumeration.
@@ -784,6 +813,14 @@ class CodeGenFunction : public CodeGenTypeCache {
/// we're currently inside a conditionally-evaluated expression.
template <class T, class... As>
void pushFullExprCleanup(CleanupKind kind, As... A) {
+ if (kind & BranchInExprCleanup) {
+ // Defer BranchInExprCleanup as a NormalCleanup (emitted only if we see
+ // a branch). Do not add these to the EHStack as they should be added
+ // separately with a different CleanupKind.
+ pushDeferredCleanup<T>(BranchInExprCleanupStack, NormalCleanup,
+ Address::invalid(), A...);
+ return;
+ }
// If we're not in a conditional branch, or if none of the
// arguments requires saving, then use the unconditional cleanup.
if (!isInConditionalBranch())
@@ -803,8 +840,8 @@ class CodeGenFunction : public CodeGenTypeCache {
template <class T, class... As>
void pushCleanupAfterFullExpr(CleanupKind Kind, As... A) {
if (!isInConditionalBranch())
- return pushCleanupAfterFullExprWithActiveFlag<T>(Kind, Address::invalid(),
- A...);
+ return pushDeferredCleanup<T>(LifetimeExtendedCleanupStack, Kind,
+ Address::invalid(), A...);
Address ActiveFlag = createCleanupActiveFlag();
assert(!DominatingValue<Address>::needsSaving(ActiveFlag) &&
@@ -814,24 +851,23 @@ class CodeGenFunction : public CodeGenTypeCache {
SavedTuple Saved{saveValueInCond(A)...};
typedef EHScopeStack::ConditionalCleanup<T, As...> CleanupType;
- pushCleanupAfterFullExprWithActiveFlag<CleanupType>(Kind, ActiveFlag, Saved);
+ pushDeferredCleanup<CleanupType>(LifetimeExtendedCleanupStack, Kind,
+ ActiveFlag, Saved);
}
template <class T, class... As>
- void pushCleanupAfterFullExprWithActiveFlag(CleanupKind Kind,
- Address ActiveFlag, As... A) {
- LifetimeExtendedCleanupHeader Header = {sizeof(T), Kind,
- ActiveFlag.isValid()};
+ void pushDeferredCleanup(DeferredCleanupStack &Cleanups, CleanupKind Kind,
+ Address ActiveFlag, As... A) {
+ DeferredCleanupHeader Header = {sizeof(T), Kind, ActiveFlag.isValid()};
- size_t OldSize = LifetimeExtendedCleanupStack.size();
- LifetimeExtendedCleanupStack.resize(
- LifetimeExtendedCleanupStack.size() + sizeof(Header) + Header.Size +
- (Header.IsConditional ? sizeof(ActiveFlag) : 0));
+ size_t OldSize = Cleanups.size();
+ Cleanups.resize(Cleanups.size() + sizeof(Header) + Header.Size +
+ (Header.IsConditional ? sizeof(ActiveFlag) : 0));
static_assert(sizeof(Header) % alignof(T) == 0,
"Cleanup will be allocated on misaligned address");
- char *Buffer = &LifetimeExtendedCleanupStack[OldSize];
- new (Buffer) LifetimeExtendedCleanupHeader(Header);
+ char *Buffer = &Cleanups[OldSize];
+ new (Buffer) DeferredCleanupHeader(Header);
new (Buffer + sizeof(Header)) T(A...);
if (Header.IsConditional)
new (Buffer + sizeof(Header) + sizeof(T)) Address(ActiveFlag);
@@ -888,6 +924,8 @@ class CodeGenFunction : public CodeGenTypeCache {
class RunCleanupsScope {
EHScopeStack::stable_iterator CleanupStackDepth, OldCleanupScopeDepth;
size_t LifetimeExtendedCleanupStackSize;
+ // size_t BranchInExprCleanupStackSize;
+ RestoreBranchInExpr RestoreBranchInExpr;
bool OldDidCallStackSave;
protected:
bool PerformCleanup;
@@ -902,11 +940,11 @@ class CodeGenFunction : public CodeGenTypeCache {
public:
/// Enter a new cleanup scope.
explicit RunCleanupsScope(CodeGenFunction &CGF)
- : PerformCleanup(true), CGF(CGF)
- {
+ : RestoreBranchInExpr(CGF), PerformCleanup(true), CGF(CGF) {
CleanupStackDepth = CGF.EHStack.stable_begin();
LifetimeExtendedCleanupStackSize =
CGF.LifetimeExtendedCleanupStack.size();
+ // BranchInExprCleanupStackSize = CGF.BranchInExprCleanupStack.size();
OldDidCallStackSave = CGF.DidCallStackSave;
CGF.DidCallStackSave = false;
OldCleanupScopeDepth = CGF.CurrentCleanupScopeDepth;
@@ -1150,9 +1188,11 @@ 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);
+ /// Adds deferred cleanups from the given position to the EHStack.
+ /// These could be lifetime-extended cleanups or branch-in-expr cleanups.
+ /// (does not remove the cleanups from the original stack).
+ void AddDeferredCleanups(DeferredCleanupStack &DeferredCleanupsStack,
+ size_t OldSize);
/// Takes the old cleanup stack size and emits the cleanup blocks
/// that have been added, then adds all lifetime-extended cleanups from
@@ -1169,8 +1209,8 @@ class CodeGenFunction : public CodeGenTypeCache {
/// to which we can perform this jump later.
JumpDest getJumpDestInCurrentScope(llvm::BasicBlock *Target) {
return JumpDest(Target, EHStack.getInnermostNormalCleanup(),
- LifetimeExtendedCleanupStack.size(),
- NextCleanupDestIndex++);
+ EHStack.getInnermostEHScope(),
+ BranchInExprCleanupStack.size(), NextCleanupDestIndex++);
}
/// The given basic block lies in the current EH scope, but may be a
@@ -2163,6 +2203,19 @@ class CodeGenFunction : public CodeGenTypeCache {
llvm_unreachable("bad destruction kind");
}
+ bool needsBranchCleanup(QualType::DestructionKind kind) {
+ switch (kind) {
+ case QualType::DK_none:
+ return false;
+ case QualType::DK_cxx_destructor:
+ case QualType::DK_objc_weak_lifetime:
+ case QualType::DK_nontrivial_c_struct:
+ case QualType::DK_objc_strong_lifetime:
+ return true;
+ }
+ llvm_unreachable("bad destruction kind");
+ }
+
CleanupKind getCleanupKind(QualType::DestructionKind kind) {
return (needsEHCleanup(kind) ? NormalAndEHCleanup : NormalCleanup);
}
diff --git a/clang/lib/CodeGen/EHScopeStack.h b/clang/lib/CodeGen/EHScopeStack.h
index 0c667e80bb6d8c..feffa37c87f6b1 100644
--- a/clang/lib/CodeGen/EHScopeStack.h
+++ b/clang/lib/CodeGen/EHScopeStack.h
@@ -85,6 +85,13 @@ enum CleanupKind : unsigned {
NormalAndEHCleanup = EHCleanup | NormalCleanup,
+ // Denotes a deferred cleanup while building an expression. These cleanups are
+ // emitted on seeing a branch in an partially built expression (eg. branches
+ // in stmt-expr and coroutine suspensions).
+ // This cleanup type should not be used with other types. Cleanups of other
+ // types should be added separately to the EHStack.
+ BranchInExprCleanup = 0x4,
+
LifetimeMarker = 0x8,
NormalEHLifetimeMarker = LifetimeMarker | NormalAndEHCleanup,
};
@@ -244,6 +251,9 @@ class EHScopeStack {
/// The innermost EH scope on the stack.
stable_iterator InnermostEHScope;
+ /// The innermost EH scope on the stack.
+ stable_iterator InnermostBranchExprCleanup;
+
/// The CGF this Stack belong to
CodeGenFunction* CGF;
diff --git a/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp b/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp
index a53b0fcf040927..ee380a167e3fbe 100644
--- a/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp
+++ b/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 --std=c++20 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s
// Context: GH63818
struct Printy {
@@ -13,6 +13,9 @@ struct Printies {
bool foo();
+// ====================================
+// Init with lifetime extensions
+// ====================================
void bar() {
// CHECK: define dso_local void @_Z3barv()
// CHECK-NOT: call void @_ZN6PrintyD1Ev
@@ -37,7 +40,9 @@ void bar() {
// CHECK-NOT: call void @_ZN6PrintyD1Ev
}
-
+// ====================================
+// Break in stmt-expr
+// ====================================
void test_break() {
// CHECK: define dso_local void @_Z10test_breakv()
// CHECK-NOT: call void @_ZN6PrintyD1Ev
@@ -66,3 +71,113 @@ void test_break() {
// CHECK-NOT: call void @_ZN6PrintyD1Ev
}
+// =============================================
+// Initialisation without lifetime-extension
+// =============================================
+void test_init_with_no_ref_binding() {
+ // CHECK: define dso_local void @_Z29test_init_with_no_ref_bindingv()
+ struct PrintiesCopy {
+ Printy a;
+ Printy b;
+ Printy c;
+ };
+ PrintiesCopy ps(Printy(), ({
+ if (foo()) {
+ // CHECK: if.then:
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label %return
+ return;
+ }
+ Printy();
+ }),
+ ({
+ if (foo()) {
+ // CHECK: if.then2:
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label %return
+ return;
+ }
+ Printy();
+ }));
+}
+
+// ====================================
+// Array init
+// ====================================
+void test_array_init() {
+ // CHECK: define dso_local void @_Z15test_array_initv()
+ Printy arr[] = {
+ Printy(),
+ ({
+ if (foo()) {
+ // CHECK: if.then:
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label %return
+ return;
+ }
+ Printy();
+ }),
+ ({
+ if (foo()) {
+ return;
+ // CHECK: if.then3:
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: call void @_ZN6PrintyD1Ev
+ // CHECK-NEXT: br label %return
+ }
+ Printy();
+ })};
+ return;
+}
+
+// ====================================
+// Arrays as subobjects
+// ====================================
+void arrays_as_subobjects() {
+ // CHECK: define dso_local void @_Z20arrays_as_subobjectsv()
+ struct S {
+ Printy arr1[2];
+ Printy arr2[2];
+ Printy p;
+ };
+ S s{{Printy(), Printy()},
+ {Printy(), ({
+ if (foo()) {
+ /** One dtor followed by an array destruction **/
+ // CHECK: if.then:
+ // CHECK: call void @_ZN6PrintyD1Ev
+ // CHECK: br label %arraydestroy.body
+
+ // CHECK: arraydestroy.body:
+ // CHECK: call void @_ZN6PrintyD1Ev
+
+ // CHECK: arraydestroy.done{{.*}}:
+ // CHECK: br label %return
+ return;
+ }
+ Printy();
+ })},
+ ({
+ if (foo()) {
+ /** Two array destructions **/
+ //CHECK: if.then{{.*}}:
+ //CHECK: br label %arraydestroy.body{{.*}}
+
+ //CHECK: arraydestroy.body{{.*}}:
+ //CHECK: call void @_ZN6PrintyD1Ev
+
+ //CHECK: arraydestroy.done{{.*}}:
+ //CHECK: br label %arraydestroy.body{{.*}}
+
+ //CHECK: arraydestroy.body{{.*}}:
+ //CHECK: call void @_ZN6PrintyD1Ev
+
+ //CHECK: arraydestroy.done{{.*}}:
+ //CHECK: br label %return
+ return;
+ }
+ Printy();
+ })};
+}
+
>From cf5024cd8a371e6a00cbe85fb0e91f476906f566 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Fri, 9 Feb 2024 20:18:04 +0000
Subject: [PATCH 9/9] remove dead code
---
clang/lib/CodeGen/CGStmt.cpp | 6 +++---
clang/lib/CodeGen/CodeGenFunction.h | 9 ++-------
clang/lib/CodeGen/EHScopeStack.h | 3 ---
3 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 9f1ab1e7817179..8d7e2616c168c0 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -627,9 +627,9 @@ CodeGenFunction::getJumpDestForLabel(const LabelDecl *D) {
if (Dest.isValid()) return Dest;
// Create, but don't insert, the new block.
- Dest = JumpDest(
- createBasicBlock(D->getName()), EHScopeStack::stable_iterator::invalid(),
- EHScopeStack::stable_iterator::invalid(), 0, NextCleanupDestIndex++);
+ Dest = JumpDest(createBasicBlock(D->getName()),
+ EHScopeStack::stable_iterator::invalid(), 0,
+ NextCleanupDestIndex++);
return Dest;
}
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 5310ec8440fd5a..1713636b554adf 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -240,15 +240,13 @@ class CodeGenFunction : public CodeGenTypeCache {
struct JumpDest {
JumpDest() : Block(nullptr), BranchInExprDepth(0), Index(0) {}
JumpDest(llvm::BasicBlock *Block, EHScopeStack::stable_iterator Depth,
- EHScopeStack::stable_iterator EHScopeDepth,
unsigned BranchInExprDepth, unsigned Index)
- : Block(Block), ScopeDepth(Depth), EHScopeDepth(EHScopeDepth),
- BranchInExprDepth(BranchInExprDepth), Index(Index) {}
+ : Block(Block), ScopeDepth(Depth), BranchInExprDepth(BranchInExprDepth),
+ Index(Index) {}
bool isValid() const { return Block != nullptr; }
llvm::BasicBlock *getBlock() const { return Block; }
EHScopeStack::stable_iterator getScopeDepth() const { return ScopeDepth; }
- EHScopeStack::stable_iterator getEHScopeDepth() const { return EHScopeDepth; }
unsigned getDestIndex() const { return Index; }
unsigned getBranchInExprDepth() const { return BranchInExprDepth; }
@@ -260,7 +258,6 @@ class CodeGenFunction : public CodeGenTypeCache {
private:
llvm::BasicBlock *Block;
EHScopeStack::stable_iterator ScopeDepth;
- EHScopeStack::stable_iterator EHScopeDepth;
// Size of the branch-in-expr cleanup stack in destination scope.
// All cleanups beyond this depth would be emitted on encountering a branch
@@ -944,7 +941,6 @@ class CodeGenFunction : public CodeGenTypeCache {
CleanupStackDepth = CGF.EHStack.stable_begin();
LifetimeExtendedCleanupStackSize =
CGF.LifetimeExtendedCleanupStack.size();
- // BranchInExprCleanupStackSize = CGF.BranchInExprCleanupStack.size();
OldDidCallStackSave = CGF.DidCallStackSave;
CGF.DidCallStackSave = false;
OldCleanupScopeDepth = CGF.CurrentCleanupScopeDepth;
@@ -1209,7 +1205,6 @@ class CodeGenFunction : public CodeGenTypeCache {
/// to which we can perform this jump later.
JumpDest getJumpDestInCurrentScope(llvm::BasicBlock *Target) {
return JumpDest(Target, EHStack.getInnermostNormalCleanup(),
- EHStack.getInnermostEHScope(),
BranchInExprCleanupStack.size(), NextCleanupDestIndex++);
}
diff --git a/clang/lib/CodeGen/EHScopeStack.h b/clang/lib/CodeGen/EHScopeStack.h
index feffa37c87f6b1..2077eb5e315fbd 100644
--- a/clang/lib/CodeGen/EHScopeStack.h
+++ b/clang/lib/CodeGen/EHScopeStack.h
@@ -251,9 +251,6 @@ class EHScopeStack {
/// The innermost EH scope on the stack.
stable_iterator InnermostEHScope;
- /// The innermost EH scope on the stack.
- stable_iterator InnermostBranchExprCleanup;
-
/// The CGF this Stack belong to
CodeGenFunction* CGF;
More information about the cfe-commits
mailing list