From 28d90711ff8e4924135d4bd4e5f252d96ac41b93 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Tue, 23 Jan 2024 20:18:25 +0000 Subject: [PATCH 01/15] [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 a0e8700b0522b..360335b86c641 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, // Matches the expression awaited by the `co_await`. AST_MATCHER_P(CoawaitExpr, awaitable, ast_matchers::internal::Matcher, 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 55a7e4b8f2954..c23c355dac1b2 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 ReturnObject raii_in_template(){ T a; From 442809ce6047f8f83c9f3f54b17182c18535bb03 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Mon, 5 Feb 2024 15:52:56 +0000 Subject: [PATCH 02/15] [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 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 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 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 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 handle; +}; + +struct coroutine::promise_type { + coroutine get_return_object() { + return {std::coroutine_handle::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 Date: Mon, 5 Feb 2024 16:52:25 +0000 Subject: [PATCH 03/15] 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 da0528b271aa3..39c65c0b07c6f 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 Date: Tue, 6 Feb 2024 14:27:40 +0000 Subject: [PATCH 04/15] 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 39c65c0b07c6f..a5a49917d108f 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 beff0ad9da270..8d7e2616c168c 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 ac55e84034bc6..78bc45dca2936 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 0000000000000..a53b0fcf04092 --- /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 214becd81e61a..0000000000000 --- 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 Date: Tue, 6 Feb 2024 14:34:25 +0000 Subject: [PATCH 05/15] 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 a5a49917d108f..e97441310e34c 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 Date: Tue, 6 Feb 2024 14:35:24 +0000 Subject: [PATCH 06/15] 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 78bc45dca2936..11e4c6481776f 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 Date: Tue, 6 Feb 2024 14:52:44 +0000 Subject: [PATCH 07/15] 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 11e4c6481776f..ec9f0de0c0338 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 Date: Fri, 9 Feb 2024 20:03:27 +0000 Subject: [PATCH 08/15] 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 e97441310e34c..af787440f8847 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( - LifetimeExtendedCleanupStack[I]); + DeferredCleanupHeader &Header = + reinterpret_cast(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
(LifetimeExtendedCleanupStack[I]); + Address ActiveFlag = reinterpret_cast
(Cleanups[I]); initFullExprCleanupWithFlag(ActiveFlag); I += sizeof(ActiveFlag); } @@ -524,8 +519,8 @@ void CodeGenFunction::PopCleanupBlocks( std::initializer_list 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 bbe14ef4c1724..66df9186155d9 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 & ~NormalCleanup), addr, type, destroyer, useEHCleanupForArray); - return pushCleanupAfterFullExprWithActiveFlag( - cleanupKind, Address::invalid(), addr, type, destroyer, useEHCleanupForArray); + pushDestroy(BranchInExprCleanup, addr, type, destroyer, + useEHCleanupForArray); + return pushDeferredCleanup( + 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( - cleanupKind, ActiveFlag, SavedAddr, type, destroyer, - useEHCleanupForArray); + pushDeferredCleanup( + 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 22f55fe9aac90..bd2e9bc0f82a9 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 8d7e2616c168c..9f1ab1e781717 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 1ad905078d349..7bff6c89de563 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 ec9f0de0c0338..5310ec8440fd5 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 NRVOFlags; EHScopeStack EHStack; - llvm::SmallVector LifetimeExtendedCleanupStack; + + using DeferredCleanupStack = llvm::SmallVector; + + // 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 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 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(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 void pushCleanupAfterFullExpr(CleanupKind Kind, As... A) { if (!isInConditionalBranch()) - return pushCleanupAfterFullExprWithActiveFlag(Kind, Address::invalid(), - A...); + return pushDeferredCleanup(LifetimeExtendedCleanupStack, Kind, + Address::invalid(), A...); Address ActiveFlag = createCleanupActiveFlag(); assert(!DominatingValue
::needsSaving(ActiveFlag) && @@ -814,24 +851,23 @@ class CodeGenFunction : public CodeGenTypeCache { SavedTuple Saved{saveValueInCond(A)...}; typedef EHScopeStack::ConditionalCleanup CleanupType; - pushCleanupAfterFullExprWithActiveFlag(Kind, ActiveFlag, Saved); + pushDeferredCleanup(LifetimeExtendedCleanupStack, Kind, + ActiveFlag, Saved); } template - 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 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 0c667e80bb6d8..feffa37c87f6b 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 a53b0fcf04092..ee380a167e3fb 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 Date: Fri, 9 Feb 2024 20:18:04 +0000 Subject: [PATCH 09/15] 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 9f1ab1e781717..8d7e2616c168c 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 5310ec8440fd5..1713636b554ad 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 feffa37c87f6b..2077eb5e315fb 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; From db43f0271b18784b5d138a442f91ef59b1c42afe Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Fri, 9 Feb 2024 21:58:54 +0000 Subject: [PATCH 10/15] format --- clang/lib/CodeGen/CGExprAgg.cpp | 2 +- clang/lib/CodeGen/CodeGenFunction.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index bd2e9bc0f82a9..9ee0b94705ba0 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -1800,7 +1800,7 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr( if (QualType::DestructionKind dtorKind = field->getType().isDestructedType()) { assert(LV.isSimple()); - if(CGF.needsBranchCleanup(dtorKind)) { + if (CGF.needsBranchCleanup(dtorKind)) { CGF.pushDestroy(BranchInExprCleanup, LV.getAddress(CGF), field->getType(), CGF.getDestroyer(dtorKind), false); } diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 1713636b554ad..74317c3815a3f 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -645,8 +645,8 @@ class CodeGenFunction : public CodeGenTypeCache { // 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. + // suspensions). Otherwise, these should be cleared the end of the expression + // and added separately to the EHStack. DeferredCleanupStack BranchInExprCleanupStack; class RestoreBranchInExpr { From 034ebda4cfd501796b82f0fe0d17141927f0a86e Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Mon, 12 Feb 2024 11:29:43 +0000 Subject: [PATCH 11/15] add branch-in-expr cleanup for lambda init --- clang/lib/CodeGen/CGExprAgg.cpp | 4 ++++ clang/lib/CodeGen/CodeGenFunction.h | 1 - .../control-flow-in-stmt-expr-cleanup.cpp | 15 +++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index 9ee0b94705ba0..9c2596d0f3009 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -1367,6 +1367,7 @@ AggExprEmitter::VisitLambdaExpr(LambdaExpr *E) { SmallVector Cleanups; llvm::Instruction *CleanupDominator = nullptr; + CodeGenFunction::RestoreBranchInExpr RestoreBranchInExpr(CGF); CXXRecordDecl::field_iterator CurField = E->getLambdaClass()->field_begin(); for (LambdaExpr::const_capture_init_iterator i = E->capture_init_begin(), e = E->capture_init_end(); @@ -1384,6 +1385,9 @@ AggExprEmitter::VisitLambdaExpr(LambdaExpr *E) { if (QualType::DestructionKind DtorKind = CurField->getType().isDestructedType()) { assert(LV.isSimple()); + if (CGF.needsBranchCleanup(DtorKind)) + CGF.pushDestroy(BranchInExprCleanup, LV.getAddress(CGF), + CurField->getType(), CGF.getDestroyer(DtorKind), false); if (CGF.needsEHCleanup(DtorKind)) { if (!CleanupDominator) CleanupDominator = CGF.Builder.CreateAlignedLoad( diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 74317c3815a3f..176441a12d4db 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -921,7 +921,6 @@ class CodeGenFunction : public CodeGenTypeCache { class RunCleanupsScope { EHScopeStack::stable_iterator CleanupStackDepth, OldCleanupScopeDepth; size_t LifetimeExtendedCleanupStackSize; - // size_t BranchInExprCleanupStackSize; RestoreBranchInExpr RestoreBranchInExpr; bool OldDidCallStackSave; protected: 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 ee380a167e3fb..fe839316021e3 100644 --- a/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp +++ b/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp @@ -181,3 +181,18 @@ void arrays_as_subobjects() { })}; } +// ==================================== +// Lambda capture initialisation +// ==================================== +void lambda_init() { + // CHECK: define dso_local void @_Z11lambda_initv() + auto S = [a = Printy(), b = ({ + if (foo()) { + return; + // CHECK: if.then: + // CHECK: call void @_ZN6PrintyD1Ev + // CHECK: br label %return + } + Printy(); + })]() {}; +} From b902c61fbcba8a4c3a3400b73c898d01eeeb115f Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Mon, 12 Feb 2024 12:31:25 +0000 Subject: [PATCH 12/15] fix var name --- clang/lib/CodeGen/CGExprAgg.cpp | 6 +++--- clang/lib/CodeGen/CodeGenFunction.h | 16 ++++++++++------ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index 9c2596d0f3009..81412de97fa46 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -582,7 +582,7 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, // elements have been initialized. llvm::Value *element = begin; - CodeGenFunction::RestoreBranchInExpr branchInExpr(CGF); + CodeGenFunction::RestoreBranchInExprRAII branchInExpr(CGF); // Emit the explicit initializers. for (uint64_t i = 0; i != NumInitElements; ++i) { // Advance to the next element. @@ -1367,7 +1367,7 @@ AggExprEmitter::VisitLambdaExpr(LambdaExpr *E) { SmallVector Cleanups; llvm::Instruction *CleanupDominator = nullptr; - CodeGenFunction::RestoreBranchInExpr RestoreBranchInExpr(CGF); + CodeGenFunction::RestoreBranchInExprRAII RestoreBranchInExpr(CGF); CXXRecordDecl::field_iterator CurField = E->getLambdaClass()->field_begin(); for (LambdaExpr::const_capture_init_iterator i = E->capture_init_begin(), e = E->capture_init_end(); @@ -1765,7 +1765,7 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr( return; } - CodeGenFunction::RestoreBranchInExpr branchInExpr(CGF); + CodeGenFunction::RestoreBranchInExprRAII 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()) { diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 176441a12d4db..a8ca5b99b6e44 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -645,15 +645,19 @@ class CodeGenFunction : public CodeGenTypeCache { // 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. + // suspensions). + // Otherwise, these should be cleared the end of the expression and added + // separately to the EHStack. DeferredCleanupStack BranchInExprCleanupStack; - class RestoreBranchInExpr { + // RAII for restoring BranchInExprCleanupStack. + // All cleanups added to this stack during its scope are simply deleted. These + // cleanups should be added to the EHStack only on emitting a branch. + class RestoreBranchInExprRAII { public: - RestoreBranchInExpr(CodeGenFunction &CGF) + RestoreBranchInExprRAII(CodeGenFunction &CGF) : CGF(CGF), OldSize(CGF.BranchInExprCleanupStack.size()) {} - ~RestoreBranchInExpr() { CGF.BranchInExprCleanupStack.resize(OldSize); } + ~RestoreBranchInExprRAII() { CGF.BranchInExprCleanupStack.resize(OldSize); } private: CodeGenFunction &CGF; @@ -921,7 +925,7 @@ class CodeGenFunction : public CodeGenTypeCache { class RunCleanupsScope { EHScopeStack::stable_iterator CleanupStackDepth, OldCleanupScopeDepth; size_t LifetimeExtendedCleanupStackSize; - RestoreBranchInExpr RestoreBranchInExpr; + RestoreBranchInExprRAII RestoreBranchInExpr; bool OldDidCallStackSave; protected: bool PerformCleanup; From 12706f231120d0bae486a161dc8df3937c28f453 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Mon, 12 Feb 2024 13:46:54 +0000 Subject: [PATCH 13/15] fix pre-existing bug --- clang/lib/CodeGen/CGExpr.cpp | 2 +- clang/lib/CodeGen/CGExprAgg.cpp | 16 ++++++---------- clang/lib/CodeGen/CGStmt.cpp | 6 ++++-- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 4a2f3caad6588..3519de8948249 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4961,7 +4961,7 @@ LValue CodeGenFunction::EmitCompoundLiteralLValue(const CompoundLiteralExpr *E){ if (QualType::DestructionKind DtorKind = E->getType().isDestructedType()) pushLifetimeExtendedDestroy(getCleanupKind(DtorKind), DeclPtr, E->getType(), getDestroyer(DtorKind), - DtorKind & EHCleanup); + getCleanupKind(DtorKind) & EHCleanup); return Result; } diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index 81412de97fa46..ba71392116bc2 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -554,7 +554,6 @@ 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 @@ -567,10 +566,6 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, elementAlign, CGF.getDestroyer(dtorKind)); cleanup = CGF.EHStack.stable_begin(); - - // Otherwise, remember that we didn't need a cleanup. - } else { - dtorKind = QualType::DK_none; } llvm::Value *one = llvm::ConstantInt::get(CGF.SizeTy, 1); @@ -599,10 +594,10 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, 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)) + // initialisation expression. + if (CGF.needsBranchCleanup(dtorKind)) CGF.pushDestroy(BranchInExprCleanup, address, elementType, - CGF.getDestroyer(myDtorKind), false /*oder true ?*/); + CGF.getDestroyer(dtorKind), false); } // Check whether there's a non-trivial array-fill expression. @@ -673,7 +668,8 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, } // Leave the partial-array cleanup if we entered one. - if (dtorKind) CGF.DeactivateCleanupBlock(cleanup, cleanupDominator); + if (cleanupDominator) + CGF.DeactivateCleanupBlock(cleanup, cleanupDominator); } //===----------------------------------------------------------------------===// @@ -717,7 +713,7 @@ AggExprEmitter::VisitCompoundLiteralExpr(CompoundLiteralExpr *E) { if (QualType::DestructionKind DtorKind = E->getType().isDestructedType()) CGF.pushLifetimeExtendedDestroy( CGF.getCleanupKind(DtorKind), Slot.getAddress(), E->getType(), - CGF.getDestroyer(DtorKind), DtorKind & EHCleanup); + CGF.getDestroyer(DtorKind), CGF.getCleanupKind(DtorKind) & EHCleanup); } /// Attempt to look through various unimportant expressions to find a diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index 8d7e2616c168c..6f40dc066796e 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -627,9 +627,11 @@ CodeGenFunction::getJumpDestForLabel(const LabelDecl *D) { if (Dest.isValid()) return Dest; // Create, but don't insert, the new block. + // FIXME: We do not know `BranchInExprDepth` for the destination and currently + // emit *all* the BranchInExpr cleanups. Dest = JumpDest(createBasicBlock(D->getName()), - EHScopeStack::stable_iterator::invalid(), 0, - NextCleanupDestIndex++); + EHScopeStack::stable_iterator::invalid(), + /*BranchInExprDepth=*/0, NextCleanupDestIndex++); return Dest; } From ed016d6a5d4f698418716834afc5c147ed95ed38 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Tue, 13 Feb 2024 13:32:08 +0000 Subject: [PATCH 14/15] fix EmitNewArrayInitializer --- clang/lib/CodeGen/CGExprAgg.cpp | 1 + clang/lib/CodeGen/CGExprCXX.cpp | 7 +++++++ .../control-flow-in-stmt-expr-cleanup.cpp | 19 +++++++++++++++++-- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index ba71392116bc2..b131c6b14b02e 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -595,6 +595,7 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, EmitInitializationToLValue(Args[i], elementLV); // Schedule to emit element cleanup if we see a branch in the array // initialisation expression. + // FIXME: Emit a single iterative cleanup instead of element-wise cleanup. if (CGF.needsBranchCleanup(dtorKind)) CGF.pushDestroy(BranchInExprCleanup, address, elementType, CGF.getDestroyer(dtorKind), false); diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index d136bfc37278f..d872b645cda41 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1117,6 +1117,7 @@ void CodeGenFunction::EmitNewArrayInitializer( Cleanup = EHStack.stable_begin(); } + RestoreBranchInExprRAII BranchInExpr(*this); CharUnits StartAlign = CurPtr.getAlignment(); unsigned i = 0; for (const Expr *IE : InitExprs) { @@ -1131,6 +1132,12 @@ void CodeGenFunction::EmitNewArrayInitializer( // initialization loops. StoreAnyExprIntoOneUnit(*this, IE, IE->getType(), CurPtr, AggValueSlot::DoesNotOverlap); + // Schedule to emit element cleanup if we see a branch in the array + // initialisation expression. + // FIXME: Emit a single iterative cleanup instead of element-wise cleanup. + if (needsBranchCleanup(DtorKind)) + pushDestroy(BranchInExprCleanup, CurPtr, ElementType, + getDestroyer(DtorKind), false); CurPtr = Address(Builder.CreateInBoundsGEP( CurPtr.getElementType(), CurPtr.getPointer(), Builder.getSize(1), "array.exp.next"), 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 fe839316021e3..6ad6ecb08e2e5 100644 --- a/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp +++ b/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp @@ -2,7 +2,9 @@ // Context: GH63818 struct Printy { - ~Printy() { } + Printy(const char *); + Printy(); + ~Printy(); }; struct Printies { @@ -131,8 +133,21 @@ void test_array_init() { return; } +void new_array_init() { + // CHECK: define dso_local void @_Z14new_array_initv() + Printy *a = new Printy[]("1", ({ + if (foo()) { + return; + // CHECK: if.then: + // CHECK-NEXT: call void @_ZN6PrintyD1Ev + } + "2"; + })); + delete[] a; +} + // ==================================== -// Arrays as subobjects +// Arrays as sub-objects // ==================================== void arrays_as_subobjects() { // CHECK: define dso_local void @_Z20arrays_as_subobjectsv() From 0001f214eb64a84b0c4aebc1ee19eefafa5f7c33 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Wed, 28 Feb 2024 05:27:11 +0000 Subject: [PATCH 15/15] [codegen] Emit EHCleanups for branch in expr --- clang/lib/CodeGen/Address.h | 5 + clang/lib/CodeGen/CGClass.cpp | 4 +- clang/lib/CodeGen/CGCleanup.cpp | 56 ++++++-- clang/lib/CodeGen/CGCleanup.h | 56 +++++++- clang/lib/CodeGen/CGCoroutine.cpp | 8 +- clang/lib/CodeGen/CGDecl.cpp | 4 +- clang/lib/CodeGen/CGExprAgg.cpp | 54 +++---- clang/lib/CodeGen/CGExprCXX.cpp | 32 +++-- clang/lib/CodeGen/CGStmt.cpp | 10 +- clang/lib/CodeGen/CodeGenFunction.cpp | 1 - clang/lib/CodeGen/CodeGenFunction.h | 91 ++++++------ .../control-flow-in-stmt-expr-cleanup.cpp | 136 ++++++++++++------ .../CodeGenCXX/cxx1y-init-captures-eh.cpp | 2 - .../CodeGenCXX/microsoft-abi-eh-cleanups.cpp | 4 - 14 files changed, 300 insertions(+), 163 deletions(-) diff --git a/clang/lib/CodeGen/Address.h b/clang/lib/CodeGen/Address.h index cf48df8f5e736..94fdf56632f21 100644 --- a/clang/lib/CodeGen/Address.h +++ b/clang/lib/CodeGen/Address.h @@ -17,6 +17,7 @@ #include "clang/AST/CharUnits.h" #include "llvm/ADT/PointerIntPair.h" #include "llvm/IR/Constants.h" +#include "llvm/IR/Instructions.h" #include "llvm/Support/MathExtras.h" namespace clang { @@ -53,6 +54,10 @@ class Address { return PointerAndKnownNonNull.getPointer(); } + llvm::AllocaInst *getAllocaInst() const { + return llvm::dyn_cast(getPointer()); + } + /// Return the type of the pointer value. llvm::PointerType *getType() const { return llvm::cast(getPointer()->getType()); diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp index 34319381901af..9f37a631e6482 100644 --- a/clang/lib/CodeGen/CGClass.cpp +++ b/clang/lib/CodeGen/CGClass.cpp @@ -671,7 +671,7 @@ static void EmitMemberInitializer(CodeGenFunction &CGF, // Ensure that we destroy the objects if an exception is thrown later in // the constructor. QualType::DestructionKind dtorKind = FieldType.isDestructedType(); - if (CGF.needsEHCleanup(dtorKind)) + if (dtorKind) CGF.pushEHDestroy(dtorKind, LHS.getAddress(CGF), FieldType); return; } @@ -710,7 +710,7 @@ void CodeGenFunction::EmitInitializerForField(FieldDecl *Field, LValue LHS, // Ensure that we destroy this object if an exception is thrown // later in the constructor. QualType::DestructionKind dtorKind = FieldType.isDestructedType(); - if (needsEHCleanup(dtorKind)) + if (dtorKind) pushEHDestroy(dtorKind, LHS.getAddress(*this), FieldType); } diff --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp index af787440f8847..8a1879dc07954 100644 --- a/clang/lib/CodeGen/CGCleanup.cpp +++ b/clang/lib/CodeGen/CGCleanup.cpp @@ -296,18 +296,22 @@ void EHScopeStack::popNullFixups() { BranchFixups.pop_back(); } -Address CodeGenFunction::createCleanupActiveFlag() { +Address CodeGenFunction::createCleanupActiveFlag(EHCleanupScope *TopCleanup) { // Create a variable to decide whether the cleanup needs to be run. Address active = CreateTempAllocaWithoutCast( Builder.getInt1Ty(), CharUnits::One(), "cleanup.cond"); - + if (TopCleanup) + TopCleanup->AddAuxInst(active.getAllocaInst()); // Initialize it to false at a site that's guaranteed to be run // before each evaluation. - setBeforeOutermostConditional(Builder.getFalse(), active); + auto *store1 = setBeforeOutermostConditional(Builder.getFalse(), active); + if (TopCleanup) + TopCleanup->AddAuxInst(store1); // Initialize it to true at the current location. - Builder.CreateStore(Builder.getTrue(), active); - + auto *store = Builder.CreateStore(Builder.getTrue(), active); + if (TopCleanup) + TopCleanup->AddAuxInst(store); return active; } @@ -674,15 +678,16 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) { Address NormalActiveFlag = Scope.shouldTestFlagInNormalCleanup() ? Scope.getActiveFlag() : Address::invalid(); - Address EHActiveFlag = - Scope.shouldTestFlagInEHCleanup() ? Scope.getActiveFlag() - : Address::invalid(); - + Address EHActiveFlag = Scope.shouldTestFlagInEHCleanup() + ? Scope.getActiveFlag() + : Address::invalid(); // Check whether we need an EH cleanup. This is only true if we've // generated a lazy EH cleanup block. llvm::BasicBlock *EHEntry = Scope.getCachedEHDispatchBlock(); assert(Scope.hasEHBranches() == (EHEntry != nullptr)); bool RequiresEHCleanup = (EHEntry != nullptr); + bool EmitEHCleanup = + RequiresEHCleanup && (EHActiveFlag.isValid() || IsActive); EHScopeStack::stable_iterator EHParent = Scope.getEnclosingEHScope(); // Check the three conditions which might require a normal cleanup: @@ -794,6 +799,8 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) { EmitSehCppScopeEnd(); } destroyOptimisticNormalEntry(*this, Scope); + if (EmitEHCleanup) + Scope.MarkEmitted(); EHStack.popCleanup(); } else { // If we have a fallthrough and no other need for the cleanup, @@ -810,6 +817,7 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) { } destroyOptimisticNormalEntry(*this, Scope); + Scope.MarkEmitted(); EHStack.popCleanup(); EmitCleanup(*this, Fn, cleanupFlags, NormalActiveFlag); @@ -946,6 +954,7 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) { } // IV. Pop the cleanup and emit it. + Scope.MarkEmitted(); EHStack.popCleanup(); assert(EHStack.hasNormalCleanups() == HasEnclosingCleanups); @@ -1049,7 +1058,7 @@ void CodeGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) { // We only actually emit the cleanup code if the cleanup is either // active or was used before it was deactivated. - if (EHActiveFlag.isValid() || IsActive) { + if (EmitEHCleanup) { cleanupFlags.setIsForEHCleanup(); EmitCleanup(*this, Fn, cleanupFlags, EHActiveFlag); } @@ -1091,6 +1100,30 @@ bool CodeGenFunction::isObviouslyBranchWithoutCleanups(JumpDest Dest) const { return false; } +void CodeGenFunction::EmitEHCleanupsForBranch(JumpDest Dest) { + if (!Dest.isValid()) + return; + EHScopeStack::stable_iterator TopCleanup = EHStack.getInnermostEHScope(); + std::vector Cleanups; + while (TopCleanup != EHStack.stable_end() && + Dest.getEHDepth().strictlyEncloses(TopCleanup)) { + if (EHStack.find(TopCleanup)->getKind() != EHScope::Cleanup) + break; + EHCleanupScope &Scope = cast(*EHStack.find(TopCleanup)); + if (!Scope.isNormalCleanup() && !Scope.shouldSkipBranchInExpr()) + Cleanups.push_back(TopCleanup); + TopCleanup = Scope.getEnclosingEHScope(); + } + for (size_t I = Cleanups.size(); I > 0; I--) { + EHCleanupScope &Scope = + cast(*EHStack.find(Cleanups[I - 1])); + assert(Scope.isEHCleanup()); + EHStack.pushCopyOfCleanup(NormalCleanup, Scope.getCleanup(), + Scope.getCleanupSize()); + cast(*EHStack.begin()) + .SetExternalAuxInsts(&Scope.getAuxillaryInstructions()); + } +} /// Terminate the current block by emitting a branch which might leave /// the current cleanup-protected scope. The target scope may not yet @@ -1109,8 +1142,7 @@ void CodeGenFunction::EmitBranchThroughCleanup(JumpDest Dest) { // expression. RunCleanupsScope BranchInExprCleanups(*this); if (Dest.isValid()) - AddDeferredCleanups(BranchInExprCleanupStack, Dest.getBranchInExprDepth()); - + EmitEHCleanupsForBranch(Dest); // Create the branch. llvm::BranchInst *BI = Builder.CreateBr(Dest.getBlock()); diff --git a/clang/lib/CodeGen/CGCleanup.h b/clang/lib/CodeGen/CGCleanup.h index fcfbf41b0eaff..ca7d0aa812e82 100644 --- a/clang/lib/CodeGen/CGCleanup.h +++ b/clang/lib/CodeGen/CGCleanup.h @@ -16,8 +16,10 @@ #include "EHScopeStack.h" #include "Address.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/IR/Instruction.h" namespace llvm { class BasicBlock; @@ -81,6 +83,8 @@ class EHScope { /// Whether the EH cleanup should test the activation flag. unsigned TestFlagInEHCleanup : 1; + unsigned SkipBranchInExpr : 1; + /// The amount of extra storage needed by the Cleanup. /// Always a multiple of the scope-stack alignment. unsigned CleanupSize : 12; @@ -256,6 +260,30 @@ class alignas(8) EHCleanupScope : public EHScope { BranchAfters; }; mutable struct ExtInfo *ExtInfo; + // Erases unused auxillary instructions for the cleanup. + // Cleanups should mark these instructions 'used' if the cleanup is + // emitted, otherwise these instructions would be erased. + struct AuxillaryInsts { + SmallVector AuxInsts; + bool used = false; + + // Records a potentially unused instruction to be erased later. + void Add(llvm::Instruction *Inst) { AuxInsts.push_back(Inst); } + + // Mark all recorded instructions as used. These will not be erased later. + void MarkUsed() { + used = true; + AuxInsts.clear(); + } + ~AuxillaryInsts() { + if (used) + return; + for (auto *Inst : llvm::reverse(AuxInsts)) + Inst->eraseFromParent(); + } + }; + mutable struct AuxillaryInsts *AuxInsts; + bool AuxInstsIsOwned = true; /// The number of fixups required by enclosing scopes (not including /// this one). If this is the top cleanup scope, all the fixups @@ -279,6 +307,19 @@ class alignas(8) EHCleanupScope : public EHScope { return sizeof(EHCleanupScope) + Size; } + AuxillaryInsts &getAuxillaryInstructions() { + if (!AuxInsts) { + assert(AuxInstsIsOwned); + AuxInsts = new struct AuxillaryInsts(); + } + return *AuxInsts; + } + + void SetExternalAuxInsts(AuxillaryInsts *NewAuxInsts) { + AuxInsts = NewAuxInsts; + AuxInstsIsOwned = false; + } + size_t getAllocatedSize() const { return sizeof(EHCleanupScope) + CleanupBits.CleanupSize; } @@ -289,7 +330,7 @@ class alignas(8) EHCleanupScope : public EHScope { EHScopeStack::stable_iterator enclosingEH) : EHScope(EHScope::Cleanup, enclosingEH), EnclosingNormal(enclosingNormal), NormalBlock(nullptr), - ActiveFlag(Address::invalid()), ExtInfo(nullptr), + ActiveFlag(Address::invalid()), ExtInfo(nullptr), AuxInsts(nullptr), FixupDepth(fixupDepth) { CleanupBits.IsNormalCleanup = isNormal; CleanupBits.IsEHCleanup = isEH; @@ -297,12 +338,16 @@ class alignas(8) EHCleanupScope : public EHScope { CleanupBits.IsLifetimeMarker = false; CleanupBits.TestFlagInNormalCleanup = false; CleanupBits.TestFlagInEHCleanup = false; + CleanupBits.SkipBranchInExpr = false; CleanupBits.CleanupSize = cleanupSize; + AuxInstsIsOwned = true; assert(CleanupBits.CleanupSize == cleanupSize && "cleanup size overflow"); } void Destroy() { + if (AuxInstsIsOwned) + delete AuxInsts; delete ExtInfo; } // Objects of EHCleanupScope are not destructed. Use Destroy(). @@ -320,6 +365,9 @@ class alignas(8) EHCleanupScope : public EHScope { bool isLifetimeMarker() const { return CleanupBits.IsLifetimeMarker; } void setLifetimeMarker() { CleanupBits.IsLifetimeMarker = true; } + void setShouldSkipBranchInExpr() { CleanupBits.SkipBranchInExpr = true; } + bool shouldSkipBranchInExpr() const { return CleanupBits.SkipBranchInExpr; } + bool hasActiveFlag() const { return ActiveFlag.isValid(); } Address getActiveFlag() const { return ActiveFlag; @@ -348,6 +396,12 @@ class alignas(8) EHCleanupScope : public EHScope { return EnclosingNormal; } + void AddAuxInst(llvm::Instruction *Inst) { + getAuxillaryInstructions().Add(Inst); + } + + void MarkEmitted() { getAuxillaryInstructions().MarkUsed(); } + size_t getCleanupSize() const { return CleanupBits.CleanupSize; } void *getCleanupBuffer() { return this + 1; } diff --git a/clang/lib/CodeGen/CGCoroutine.cpp b/clang/lib/CodeGen/CGCoroutine.cpp index 888d30bfb3e1d..5f0dc3e95cdbf 100644 --- a/clang/lib/CodeGen/CGCoroutine.cpp +++ b/clang/lib/CodeGen/CGCoroutine.cpp @@ -12,9 +12,10 @@ #include "CGCleanup.h" #include "CodeGenFunction.h" -#include "llvm/ADT/ScopeExit.h" +#include "EHScopeStack.h" #include "clang/AST/StmtCXX.h" #include "clang/AST/StmtVisitor.h" +#include "llvm/ADT/ScopeExit.h" using namespace clang; using namespace CodeGen; @@ -743,6 +744,11 @@ void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt &S) { GroManager.EmitGroInit(); EHStack.pushCleanup(EHCleanup); + { + EHCleanupScope &Scope = + cast(*EHStack.find(EHStack.stable_begin())); + Scope.setShouldSkipBranchInExpr(); + } CurCoro.Data->CurrentAwaitKind = AwaitKind::Init; CurCoro.Data->ExceptionHandler = S.getExceptionHandler(); diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp index 66df9186155d9..1de0fe0072cae 100644 --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -34,9 +34,12 @@ #include "clang/Basic/TargetInfo.h" #include "clang/CodeGen/CGFunctionInfo.h" #include "clang/Sema/Sema.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/Analysis/ValueTracking.h" #include "llvm/IR/DataLayout.h" #include "llvm/IR/GlobalVariable.h" +#include "llvm/IR/Instruction.h" #include "llvm/IR/Intrinsics.h" #include "llvm/IR/Type.h" #include @@ -2164,7 +2167,6 @@ CodeGenFunction::getDestroyer(QualType::DestructionKind kind) { void CodeGenFunction::pushEHDestroy(QualType::DestructionKind dtorKind, Address addr, QualType type) { assert(dtorKind && "cannot push destructor for trivial type"); - assert(needsEHCleanup(dtorKind)); pushDestroy(EHCleanup, addr, type, getDestroyer(dtorKind), true); } diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index b131c6b14b02e..c9077b22873a2 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -22,9 +22,13 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/StmtVisitor.h" +#include "llvm/ADT/SmallVector.h" +#include "llvm/IR/BasicBlock.h" #include "llvm/IR/Constants.h" #include "llvm/IR/Function.h" #include "llvm/IR/GlobalVariable.h" +#include "llvm/IR/Instruction.h" +#include "llvm/IR/Instructions.h" #include "llvm/IR/IntrinsicInst.h" #include "llvm/IR/Intrinsics.h" using namespace clang; @@ -552,20 +556,30 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, // For that, we'll need an EH cleanup. QualType::DestructionKind dtorKind = elementType.isDestructedType(); Address endOfInit = Address::invalid(); + EHCleanupScope *arrayCleanup = nullptr; EHScopeStack::stable_iterator cleanup; llvm::Instruction *cleanupDominator = nullptr; - if (CGF.needsEHCleanup(dtorKind)) { + if (dtorKind) { // In principle we could tell the cleanup where we are more // directly, but the control flow can get so varied here that it // would actually be quite complex. Therefore we go through an // alloca. - endOfInit = CGF.CreateTempAlloca(begin->getType(), CGF.getPointerAlign(), - "arrayinit.endOfInit"); + Address AllocaInst = Address::invalid(); + endOfInit = + CGF.CreateTempAlloca(begin->getType(), CGF.getPointerAlign(), + "arrayinit.endOfInit", nullptr, &AllocaInst); cleanupDominator = Builder.CreateStore(begin, endOfInit); CGF.pushIrregularPartialArrayCleanup(begin, endOfInit, elementType, elementAlign, CGF.getDestroyer(dtorKind)); cleanup = CGF.EHStack.stable_begin(); + arrayCleanup = + &cast(*CGF.EHStack.find(CGF.EHStack.stable_begin())); + arrayCleanup->AddAuxInst(AllocaInst.getAllocaInst()); + if (auto *AddrSpaceCast = + llvm::dyn_cast(endOfInit.getPointer())) + arrayCleanup->AddAuxInst(AddrSpaceCast); + arrayCleanup->AddAuxInst(cleanupDominator); } llvm::Value *one = llvm::ConstantInt::get(CGF.SizeTy, 1); @@ -577,7 +591,6 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, // elements have been initialized. llvm::Value *element = begin; - CodeGenFunction::RestoreBranchInExprRAII branchInExpr(CGF); // Emit the explicit initializers. for (uint64_t i = 0; i != NumInitElements; ++i) { // Advance to the next element. @@ -588,17 +601,12 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, // Tell the cleanup that it needs to destroy up to this // element. TODO: some of these stores can be trivially // observed to be unnecessary. - if (endOfInit.isValid()) Builder.CreateStore(element, endOfInit); + if (endOfInit.isValid()) + arrayCleanup->AddAuxInst(Builder.CreateStore(element, endOfInit)); } 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 expression. - // FIXME: Emit a single iterative cleanup instead of element-wise cleanup. - if (CGF.needsBranchCleanup(dtorKind)) - CGF.pushDestroy(BranchInExprCleanup, address, elementType, - CGF.getDestroyer(dtorKind), false); } // Check whether there's a non-trivial array-fill expression. @@ -618,7 +626,8 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, if (NumInitElements) { element = Builder.CreateInBoundsGEP( llvmElementType, element, one, "arrayinit.start"); - if (endOfInit.isValid()) Builder.CreateStore(element, endOfInit); + if (endOfInit.isValid()) + arrayCleanup->AddAuxInst(Builder.CreateStore(element, endOfInit)); } // Compute the end of the array. @@ -656,7 +665,8 @@ void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType, llvmElementType, currentElement, one, "arrayinit.next"); // Tell the EH cleanup that we finished with the last element. - if (endOfInit.isValid()) Builder.CreateStore(nextElement, endOfInit); + if (endOfInit.isValid()) + arrayCleanup->AddAuxInst(Builder.CreateStore(nextElement, endOfInit)); // Leave the loop if we're done. llvm::Value *done = Builder.CreateICmpEQ(nextElement, end, @@ -1364,7 +1374,6 @@ AggExprEmitter::VisitLambdaExpr(LambdaExpr *E) { SmallVector Cleanups; llvm::Instruction *CleanupDominator = nullptr; - CodeGenFunction::RestoreBranchInExprRAII RestoreBranchInExpr(CGF); CXXRecordDecl::field_iterator CurField = E->getLambdaClass()->field_begin(); for (LambdaExpr::const_capture_init_iterator i = E->capture_init_begin(), e = E->capture_init_end(); @@ -1382,10 +1391,7 @@ AggExprEmitter::VisitLambdaExpr(LambdaExpr *E) { if (QualType::DestructionKind DtorKind = CurField->getType().isDestructedType()) { assert(LV.isSimple()); - if (CGF.needsBranchCleanup(DtorKind)) - CGF.pushDestroy(BranchInExprCleanup, LV.getAddress(CGF), - CurField->getType(), CGF.getDestroyer(DtorKind), false); - if (CGF.needsEHCleanup(DtorKind)) { + if (DtorKind) { if (!CleanupDominator) CleanupDominator = CGF.Builder.CreateAlignedLoad( CGF.Int8Ty, @@ -1762,7 +1768,7 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr( return; } - CodeGenFunction::RestoreBranchInExprRAII 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()) { @@ -1797,25 +1803,19 @@ void AggExprEmitter::VisitCXXParenListOrInitListExpr( // Push a destructor if necessary. // FIXME: if we have an array of structures, all explicitly // initialized, we can end up pushing a linear number of cleanups. - bool pushedCleanup = false; 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)) { + if (dtorKind) { CGF.pushDestroy(EHCleanup, LV.getAddress(CGF), field->getType(), CGF.getDestroyer(dtorKind), false); addCleanup(CGF.EHStack.stable_begin()); - pushedCleanup = true; } } // If the GEP didn't get used because of a dead zero init or something // else, clean it up for -O0 builds and general tidiness. - if (!pushedCleanup && LV.isSimple()) + if (!cleanupDominator && LV.isSimple()) if (llvm::GetElementPtrInst *GEP = dyn_cast(LV.getPointer(CGF))) if (GEP->use_empty()) diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index d872b645cda41..b88e692fc250a 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -16,9 +16,11 @@ #include "CGObjCRuntime.h" #include "CodeGenFunction.h" #include "ConstantEmitter.h" +#include "EHScopeStack.h" #include "TargetInfo.h" #include "clang/Basic/CodeGenOptions.h" #include "clang/CodeGen/CGFunctionInfo.h" +#include "llvm/IR/Instruction.h" #include "llvm/IR/Intrinsics.h" using namespace clang; @@ -1004,6 +1006,8 @@ void CodeGenFunction::EmitNewArrayInitializer( const Expr *Init = E->getInitializer(); Address EndOfInit = Address::invalid(); + + EHCleanupScope *ArrayCleanup = nullptr; QualType::DestructionKind DtorKind = ElementType.isDestructedType(); EHScopeStack::stable_iterator Cleanup; llvm::Instruction *CleanupDominator = nullptr; @@ -1103,21 +1107,28 @@ void CodeGenFunction::EmitNewArrayInitializer( } // Enter a partial-destruction Cleanup if necessary. - if (needsEHCleanup(DtorKind)) { + if (DtorKind) { // In principle we could tell the Cleanup where we are more // directly, but the control flow can get so varied here that it // would actually be quite complex. Therefore we go through an // alloca. + Address AllocaInst = Address::invalid(); EndOfInit = CreateTempAlloca(BeginPtr.getType(), getPointerAlign(), - "array.init.end"); + "array.init.end", nullptr, &AllocaInst); CleanupDominator = Builder.CreateStore(BeginPtr.getPointer(), EndOfInit); pushIrregularPartialArrayCleanup(BeginPtr.getPointer(), EndOfInit, ElementType, ElementAlign, getDestroyer(DtorKind)); Cleanup = EHStack.stable_begin(); + ArrayCleanup = + &cast(*EHStack.find(EHStack.stable_begin())); + ArrayCleanup->AddAuxInst(AllocaInst.getAllocaInst()); + if (auto *AddrSpaceCast = + llvm::dyn_cast(EndOfInit.getPointer())) + ArrayCleanup->AddAuxInst(AddrSpaceCast); + ArrayCleanup->AddAuxInst(CleanupDominator); } - RestoreBranchInExprRAII BranchInExpr(*this); CharUnits StartAlign = CurPtr.getAlignment(); unsigned i = 0; for (const Expr *IE : InitExprs) { @@ -1125,19 +1136,14 @@ void CodeGenFunction::EmitNewArrayInitializer( // element. TODO: some of these stores can be trivially // observed to be unnecessary. if (EndOfInit.isValid()) { - Builder.CreateStore(CurPtr.getPointer(), EndOfInit); + ArrayCleanup->AddAuxInst( + Builder.CreateStore(CurPtr.getPointer(), EndOfInit)); } // FIXME: If the last initializer is an incomplete initializer list for // an array, and we have an array filler, we can fold together the two // initialization loops. StoreAnyExprIntoOneUnit(*this, IE, IE->getType(), CurPtr, AggValueSlot::DoesNotOverlap); - // Schedule to emit element cleanup if we see a branch in the array - // initialisation expression. - // FIXME: Emit a single iterative cleanup instead of element-wise cleanup. - if (needsBranchCleanup(DtorKind)) - pushDestroy(BranchInExprCleanup, CurPtr, ElementType, - getDestroyer(DtorKind), false); CurPtr = Address(Builder.CreateInBoundsGEP( CurPtr.getElementType(), CurPtr.getPointer(), Builder.getSize(1), "array.exp.next"), @@ -1194,7 +1200,8 @@ void CodeGenFunction::EmitNewArrayInitializer( // FIXME: Share this cleanup with the constructor call emission rather than // having it create a cleanup of its own. if (EndOfInit.isValid()) - Builder.CreateStore(CurPtr.getPointer(), EndOfInit); + ArrayCleanup->AddAuxInst( + Builder.CreateStore(CurPtr.getPointer(), EndOfInit)); // Emit a constructor call loop to initialize the remaining elements. if (InitListElements) @@ -1281,7 +1288,8 @@ void CodeGenFunction::EmitNewArrayInitializer( // Store the new Cleanup position for irregular Cleanups. if (EndOfInit.isValid()) - Builder.CreateStore(CurPtr.getPointer(), EndOfInit); + ArrayCleanup->AddAuxInst( + Builder.CreateStore(CurPtr.getPointer(), EndOfInit)); // Enter a partial-destruction Cleanup if necessary. if (!CleanupDominator && needsEHCleanup(DtorKind)) { diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index 6f40dc066796e..78dc295b568e5 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -627,11 +627,11 @@ CodeGenFunction::getJumpDestForLabel(const LabelDecl *D) { if (Dest.isValid()) return Dest; // Create, but don't insert, the new block. - // FIXME: We do not know `BranchInExprDepth` for the destination and currently - // emit *all* the BranchInExpr cleanups. - Dest = JumpDest(createBasicBlock(D->getName()), - EHScopeStack::stable_iterator::invalid(), - /*BranchInExprDepth=*/0, NextCleanupDestIndex++); + // FIXME: We do not know `EHDepth` for the destination and currently + // emit *all* the EHCleanups when a branch in expr is encountered. + Dest = JumpDest( + createBasicBlock(D->getName()), EHScopeStack::stable_iterator::invalid(), + EHScopeStack::stable_iterator::invalid(), NextCleanupDestIndex++); return Dest; } diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 7bff6c89de563..ebbc38bef2ab8 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -333,7 +333,6 @@ void CodeGenFunction::FinishFunction(SourceLocation EndLoc) { "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 a8ca5b99b6e44..2d436299f44d4 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -14,6 +14,7 @@ #define LLVM_CLANG_LIB_CODEGEN_CODEGENFUNCTION_H #include "CGBuilder.h" +#include "CGCleanup.h" #include "CGDebugInfo.h" #include "CGLoopInfo.h" #include "CGValue.h" @@ -238,17 +239,16 @@ 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), BranchInExprDepth(0), Index(0) {} + JumpDest() : Block(nullptr), Index(0) {} JumpDest(llvm::BasicBlock *Block, EHScopeStack::stable_iterator Depth, - unsigned BranchInExprDepth, unsigned Index) - : Block(Block), ScopeDepth(Depth), BranchInExprDepth(BranchInExprDepth), - Index(Index) {} + EHScopeStack::stable_iterator EHDepth, unsigned Index) + : Block(Block), ScopeDepth(Depth), EHDepth(EHDepth), Index(Index) {} bool isValid() const { return Block != nullptr; } llvm::BasicBlock *getBlock() const { return Block; } EHScopeStack::stable_iterator getScopeDepth() const { return ScopeDepth; } + EHScopeStack::stable_iterator getEHDepth() const { return EHDepth; } unsigned getDestIndex() const { return Index; } - unsigned getBranchInExprDepth() const { return BranchInExprDepth; } // This should be used cautiously. void setScopeDepth(EHScopeStack::stable_iterator depth) { @@ -258,11 +258,8 @@ class CodeGenFunction : public CodeGenTypeCache { private: llvm::BasicBlock *Block; EHScopeStack::stable_iterator ScopeDepth; + EHScopeStack::stable_iterator EHDepth; - // 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; }; @@ -641,28 +638,31 @@ class CodeGenFunction : public CodeGenTypeCache { // 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; + // // 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; // RAII for restoring BranchInExprCleanupStack. // All cleanups added to this stack during its scope are simply deleted. These // cleanups should be added to the EHStack only on emitting a branch. - class RestoreBranchInExprRAII { - public: - RestoreBranchInExprRAII(CodeGenFunction &CGF) - : CGF(CGF), OldSize(CGF.BranchInExprCleanupStack.size()) {} - ~RestoreBranchInExprRAII() { CGF.BranchInExprCleanupStack.resize(OldSize); } - - private: - CodeGenFunction &CGF; - size_t OldSize; - }; + // class RestoreBranchInExprRAII { + // public: + // RestoreBranchInExprRAII(CodeGenFunction &CGF) + // : CGF(CGF), OldSize(CGF.BranchInExprCleanupStack.size()) {} + // ~RestoreBranchInExprRAII() { + // CGF.BranchInExprCleanupStack.resize(OldSize); } + + // private: + // CodeGenFunction &CGF; + // size_t OldSize; + // }; llvm::SmallVector SEHTryEpilogueStack; @@ -818,8 +818,6 @@ class CodeGenFunction : public CodeGenTypeCache { // 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(BranchInExprCleanupStack, NormalCleanup, - Address::invalid(), A...); return; } // If we're not in a conditional branch, or if none of the @@ -833,7 +831,10 @@ class CodeGenFunction : public CodeGenTypeCache { typedef EHScopeStack::ConditionalCleanup CleanupType; EHStack.pushCleanupTuple(kind, Saved); - initFullExprCleanup(); + EHCleanupScope *TopCleanup = nullptr; + if (EHStack.begin()->getKind() == EHScope::Kind::Cleanup) + TopCleanup = &cast(*EHStack.begin()); + initFullExprCleanupWithFlag(createCleanupActiveFlag(TopCleanup)); } /// Queue a cleanup to be pushed after finishing the current full-expression, @@ -881,7 +882,9 @@ class CodeGenFunction : public CodeGenTypeCache { } void initFullExprCleanupWithFlag(Address ActiveFlag); - Address createCleanupActiveFlag(); + // Also attaches the produced instructions to the cleanup auxillary + // instructions. + Address createCleanupActiveFlag(EHCleanupScope *TopCleanup = nullptr); /// PushDestructorCleanup - Push a cleanup to call the /// complete-object destructor of an object of the given type at the @@ -925,7 +928,6 @@ class CodeGenFunction : public CodeGenTypeCache { class RunCleanupsScope { EHScopeStack::stable_iterator CleanupStackDepth, OldCleanupScopeDepth; size_t LifetimeExtendedCleanupStackSize; - RestoreBranchInExprRAII RestoreBranchInExpr; bool OldDidCallStackSave; protected: bool PerformCleanup; @@ -940,7 +942,7 @@ class CodeGenFunction : public CodeGenTypeCache { public: /// Enter a new cleanup scope. explicit RunCleanupsScope(CodeGenFunction &CGF) - : RestoreBranchInExpr(CGF), PerformCleanup(true), CGF(CGF) { + : PerformCleanup(true), CGF(CGF) { CleanupStackDepth = CGF.EHStack.stable_begin(); LifetimeExtendedCleanupStackSize = CGF.LifetimeExtendedCleanupStack.size(); @@ -1208,7 +1210,7 @@ class CodeGenFunction : public CodeGenTypeCache { /// to which we can perform this jump later. JumpDest getJumpDestInCurrentScope(llvm::BasicBlock *Target) { return JumpDest(Target, EHStack.getInnermostNormalCleanup(), - BranchInExprCleanupStack.size(), NextCleanupDestIndex++); + EHStack.getInnermostEHScope(), NextCleanupDestIndex++); } /// The given basic block lies in the current EH scope, but may be a @@ -1222,7 +1224,7 @@ class CodeGenFunction : public CodeGenTypeCache { /// block through the normal cleanup handling code (if any) and then /// on to \arg Dest. void EmitBranchThroughCleanup(JumpDest Dest); - + void EmitEHCleanupsForBranch(JumpDest Dest); /// isObviouslyBranchWithoutCleanups - Return true if a branch to the /// specified destination obviously has no cleanups to run. 'false' is always /// a conservatively correct answer for this method. @@ -1269,11 +1271,13 @@ class CodeGenFunction : public CodeGenTypeCache { /// one branch or the other of a conditional expression. bool isInConditionalBranch() const { return OutermostConditional != nullptr; } - void setBeforeOutermostConditional(llvm::Value *value, Address addr) { + llvm::StoreInst *setBeforeOutermostConditional(llvm::Value *value, + Address addr) { assert(isInConditionalBranch()); llvm::BasicBlock *block = OutermostConditional->getStartingBlock(); - auto store = new llvm::StoreInst(value, addr.getPointer(), &block->back()); + auto *store = new llvm::StoreInst(value, addr.getPointer(), &block->back()); store->setAlignment(addr.getAlignment().getAsAlign()); + return store; } /// An RAII object to record that we're evaluating a statement @@ -2201,19 +2205,6 @@ 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/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp b/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp index 6ad6ecb08e2e5..310de6ce15d0c 100644 --- a/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp +++ b/clang/test/CodeGenCXX/control-flow-in-stmt-expr-cleanup.cpp @@ -94,7 +94,7 @@ void test_init_with_no_ref_binding() { }), ({ if (foo()) { - // CHECK: if.then2: + // CHECK: if.then{{.*}}: // CHECK-NEXT: call void @_ZN6PrintyD1Ev // CHECK-NEXT: call void @_ZN6PrintyD1Ev // CHECK-NEXT: br label %return @@ -111,10 +111,30 @@ void test_array_init() { // CHECK: define dso_local void @_Z15test_array_initv() Printy arr[] = { Printy(), + // CHECK: entry: + // CHECK: %arrayinit.endOfInit = alloca ptr, align 8 + // CHECK-NEXT: %arrayinit.begin = getelementptr inbounds + // CHECK-NEXT: store ptr %arrayinit.begin, ptr %arrayinit.endOfInit, align 8 + // CHECK-NEXT: call void @_ZN6PrintyC1Ev + // CHECK-NEXT: %arrayinit.element = getelementptr inbounds %struct.Printy, ptr %arrayinit.begin, i64 1 + // CHECK-NEXT: store ptr %arrayinit.element, ptr %arrayinit.endOfInit, align 8 + // CHECK-NEXT: @_Z3foov() + // CHECK-NEXT: br i1 %call, label %if.then, label %if.end ({ if (foo()) { // CHECK: if.then: + // CHECK-NEXT: load ptr, ptr %arrayinit.endOfInit, + // CHECK-NEXT: %arraydestroy.isempty = icmp eq ptr %arrayinit.begin, {{.*}} + // CHECK-NEXT: br i1 %arraydestroy.isempty, label %arraydestroy.done{{.*}}, label %arraydestroy.body + + // CHECK: arraydestroy.body: + // CHECK-NEXT: %arraydestroy.elementPast = phi ptr [ %0, %if.then ], [ %arraydestroy.element, %arraydestroy.body ] + // CHECK-NEXT: %arraydestroy.element = getelementptr inbounds %struct.Printy, ptr %arraydestroy.elementPast, i64 -1 // CHECK-NEXT: call void @_ZN6PrintyD1Ev + // CHECK-NEXT: %arraydestroy.done = icmp eq ptr %arraydestroy.element, %arrayinit.begin + // CHECK-NEXT: br i1 %arraydestroy.done, label %arraydestroy.done{{.*}}, label %arraydestroy.body + + // CHECK: arraydestroy.done{{.*}}: // CHECK-NEXT: br label %return return; } @@ -123,9 +143,19 @@ void test_array_init() { ({ if (foo()) { return; - // CHECK: if.then3: - // CHECK-NEXT: call void @_ZN6PrintyD1Ev + // CHECK: if.then{{.*}}: + // CHECK-NEXT: load ptr, ptr %arrayinit.endOfInit + // CHECK-NEXT: %arraydestroy.isempty{{.*}} = icmp eq ptr %arrayinit.begin, {{.*}} + // CHECK-NEXT: br i1 %arraydestroy.isempty{{.*}}, label %arraydestroy.done{{.*}}, label %arraydestroy.body{{.*}} + + // CHECK: arraydestroy.body{{.*}}: + // CHECK-NEXT: %arraydestroy.elementPast{{.*}} = phi ptr [ %1, %if.then{{.*}} ], [ %arraydestroy.element{{.*}}, %arraydestroy.body{{.*}} ] + // CHECK-NEXT: %arraydestroy.element{{.*}} = getelementptr inbounds %struct.Printy, ptr %arraydestroy.elementPast{{.*}}, i64 -1 // CHECK-NEXT: call void @_ZN6PrintyD1Ev + // CHECK-NEXT: %arraydestroy.done{{.*}} = icmp eq ptr %arraydestroy.element{{.*}}, %arrayinit.begin + // CHECK-NEXT: br i1 %arraydestroy.done{{.*}}, label %arraydestroy.done{{.*}}, label %arraydestroy.body{{.*}} + + // CHECK: arraydestroy.done{{.*}}: // CHECK-NEXT: br label %return } Printy(); @@ -135,65 +165,81 @@ void test_array_init() { void new_array_init() { // CHECK: define dso_local void @_Z14new_array_initv() - Printy *a = new Printy[]("1", ({ - if (foo()) { - return; - // CHECK: if.then: - // CHECK-NEXT: call void @_ZN6PrintyD1Ev - } - "2"; - })); + Printy *a = new Printy[]("1", + // CHECK: entry: + // CHECK: %array.init.end = alloca ptr, align 8 + // CHECK: store ptr %0, ptr %array.init.end, align 8 + // CHECK-NEXT: store ptr %0, ptr %array.init.end, align 8 + // CHECK: store ptr %array.exp.next, ptr %array.init.end, align 8 + ({ + if (foo()) { + return; + // CHECK: if.then{{.*}}: + // CHECK-NEXT: load ptr, ptr %array.init.end, align 8 + // CHECK-NEXT: %arraydestroy.isempty = icmp + // CHECK-NEXT: br i1 %arraydestroy.isempty, label %arraydestroy.done{{.*}}, label %arraydestroy.body + + // CHECK: arraydestroy.body: + // CHECK-NEXT: %arraydestroy.elementPast = phi ptr [ %{{.*}}, %if.then ], [ %arraydestroy.element, %arraydestroy.body ] + // CHECK-NEXT: %arraydestroy.element = getelementptr inbounds %struct.Printy, ptr %arraydestroy.elementPast, i64 -1 + // CHECK-NEXT: call void @_ZN6PrintyD1Ev + // CHECK-NEXT: %arraydestroy.done = icmp eq ptr %arraydestroy.element, %0 + // CHECK-NEXT: br i1 %arraydestroy.done, label %arraydestroy.done2, label %arraydestroy.body + + // CHECK: arraydestroy.done{{.*}}: + // CHECK-NEXT: br label %delete.end + } + "2"; + })); + // CHECK: delete.end{{.*}}: + // CHECK-NEXT: ret void delete[] a; } // ==================================== // Arrays as sub-objects // ==================================== + 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(), ({ + S s{{Printy(), + // CHECK: %arrayinit.endOfInit = alloca ptr, align 8 + // CHECK: %arrayinit.endOfInit{{.*}} = alloca ptr, align 8 + // CHECK: store ptr %arrayinit.begin, ptr %arrayinit.endOfInit + // CHECK: call void @_ZN6PrintyC1Ev + // CHECK: store ptr %arrayinit.element, ptr %arrayinit.endOfInit + ({ 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; + // CHECK: if.then{{.*}}: + // CHECK-NEXT: load ptr, ptr %arrayinit.endOfInit + // CHECK: arraydestroy.body: + // CHECK: arraydestroy.done1: + // CHECK-NEXT: br label %return } + // CHECK: if.end: + // CHECK: call void @_ZN6PrintyC1Ev + // CHECK: store ptr %arrayinit.element4, ptr %arrayinit.endOfInit3, align 8 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(); - })}; + {Printy(), ({ + if (foo()) { + /** One dtor followed by an array destruction **/ + // CHECK: if.then{{.*}}: + // CHECK-NEXT: load ptr, ptr %arrayinit.endOfInit3 + // CHECK: arraydestroy.body{{.*}}: + // CHECK: arraydestroy.done{{.*}}: + // CHECK: arraydestroy.body{{.*}}: + // CHECK: arraydestroy.done{{.*}}: + // CHECK-NEXT: br label %return + return; + } + Printy(); + })}}; } // ==================================== diff --git a/clang/test/CodeGenCXX/cxx1y-init-captures-eh.cpp b/clang/test/CodeGenCXX/cxx1y-init-captures-eh.cpp index b0b938191a023..12cf4e42b9871 100644 --- a/clang/test/CodeGenCXX/cxx1y-init-captures-eh.cpp +++ b/clang/test/CodeGenCXX/cxx1y-init-captures-eh.cpp @@ -50,9 +50,7 @@ void y() noexcept; // CHECK-LABEL: define{{.*}} void @_Z1hbb( void h(bool b1, bool b2) { - // CHECK: {{.*}} = alloca i1, // CHECK: %[[S_ISACTIVE:.*]] = alloca i1, - // CHECK: {{.*}} = alloca i1, // lambda init: s and t, branch on b1 // CHECK: call void @_ZN1SC1Ev( diff --git a/clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp b/clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp index f00cf90762756..3c643039e848c 100644 --- a/clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp +++ b/clang/test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp @@ -124,7 +124,6 @@ int HasConditionalDeactivatedCleanups(bool cond) { // WIN32-O0: store i1 false // WIN32-O0: store i1 false // WIN32-O0: store i1 false -// WIN32-O0: store i1 false // WIN32-O0: br i1 // True condition. // WIN32-O0: call x86_thiscallcc noundef ptr @"??0A@@QAE@XZ" @@ -136,7 +135,6 @@ int HasConditionalDeactivatedCleanups(bool cond) { // WIN32-O0: store i1 true // WIN32-O0: invoke void @"?TakeRef@@YAXABUA@@@Z" // WIN32-O0: invoke x86_thiscallcc noundef ptr @"??0A@@QAE@XZ" -// WIN32-O0: store i1 true // WIN32-O0: store i1 false, ptr %[[arg1_cond]] // WIN32-O0: invoke noundef i32 @"?TakesTwo@@YAHUA@@0@Z" // False condition. @@ -162,7 +160,6 @@ int HasConditionalDeactivatedCleanups(bool cond) { // WIN32-O3: store i1 false // WIN32-O3: store i1 false // WIN32-O3: store i1 false -// WIN32-O3: store i1 false // WIN32-O3: br i1 // True condition. // WIN32-O3: call x86_thiscallcc noundef ptr @"??0A@@QAE@XZ" @@ -174,7 +171,6 @@ int HasConditionalDeactivatedCleanups(bool cond) { // WIN32-O3: store i1 true // WIN32-O3: invoke void @"?TakeRef@@YAXABUA@@@Z" // WIN32-O3: invoke x86_thiscallcc noundef ptr @"??0A@@QAE@XZ" -// WIN32-O3: store i1 true // WIN32-O3: store i1 false, ptr %[[arg1_cond]] // WIN32-O3: invoke noundef i32 @"?TakesTwo@@YAHUA@@0@Z" // False condition.