llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clangir
Author: Morris Hafner (mmha)
<details>
<summary>Changes</summary>
The lifetime of temporaries used to initialize a loop condition variable needs
to be extended to the end of each iteration. Example:
```c++
ConvertibleToBool get_ctb();
while(ConvertibleToBool const& cond = get_ctb()) {
...
} // destroy temporary here, then reevaluate
```
Capture the destructor cleanups so temporary lifetimes get extended.
Assisted-By: Claude Opus 5.5 <noreply@<!-- -->anthropic.com>
---
Patch is 44.84 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/231469.diff
7 Files Affected:
- (modified) clang/lib/CIR/CodeGen/CIRGenCleanup.cpp (+4-3)
- (modified) clang/lib/CIR/CodeGen/CIRGenDecl.cpp (+127-12)
- (modified) clang/lib/CIR/CodeGen/CIRGenExpr.cpp (+2-1)
- (modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+30-2)
- (modified) clang/lib/CIR/CodeGen/CIRGenStmt.cpp (+20-6)
- (modified) clang/test/CIR/CodeGen/lifetime-marker.cpp (+77-4)
- (modified) clang/test/CIR/CodeGen/loop-cond-var-cleanup.cpp (+495)
``````````diff
diff --git a/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp
b/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp
index 7a703b84bb04f..cc94983c52899 100644
--- a/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp
@@ -833,10 +833,11 @@ void
CIRGenFunction::emitCapturedCleanups(EHScopeStack::stable_iterator depth,
if (scope.isEHCleanup())
cleanupFlags.setIsEHCleanupKind();
- // A captured cleanup may be guarded by an active flag. A loop condition
- // variable's destructor cleanup has one that is false while its
+ // A captured cleanup may be guarded by an active flag. The destructor
+ // cleanups of a loop condition variable and of the temporaries that its
+ // initializer lifetime-extends have one that is false while its
// initializer runs, so a throwing initializer does not destroy the
- // not-yet-constructed variable. The lifetime-end cleanup has no flag
+ // not-yet-constructed variables. The lifetime-end cleanup has no flag
// because its lifetime starts before initialization. For a loop, each
// emission serves both the normal per-iteration exit and the EH unwind
// path.
diff --git a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
index 01b9f810fbed4..000792eba7358 100644
--- a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
@@ -22,10 +22,12 @@
#include "clang/AST/DeclOpenACC.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/Basic/Cuda.h"
#include "clang/CIR/Dialect/IR/CIRAttrs.h"
#include "clang/CIR/Dialect/IR/CIRDialect.h"
#include "clang/CIR/MissingFeatures.h"
+#include "llvm/Support/SaveAndRestore.h"
using namespace clang;
using namespace clang::CIRGen;
@@ -387,13 +389,76 @@ void CIRGenFunction::emitAutoVarDecl(const VarDecl &d) {
emitAutoVarCleanups(emission);
}
-void CIRGenFunction::emitLoopConditionVariable(const VarDecl &d,
- CapturedCleanups &condCleanup) {
+namespace {
+/// Finds the temporaries with a destructor that the initializer of a variable
+/// with automatic storage duration lifetime-extends. Their cleanups are pushed
+/// onto the EH stack when the initializer's full-expression ends.
+class LifetimeExtendedTemporaryFinder
+ : public RecursiveASTVisitor<LifetimeExtendedTemporaryFinder> {
+ const VarDecl &var;
+ llvm::SmallPtrSet<const MaterializeTemporaryExpr *, 2> visited;
+ SmallVector<QualType, 2> types;
+
+public:
+ explicit LifetimeExtendedTemporaryFinder(const VarDecl &var) : var(var) {}
+
+ /// The types of the temporaries that were found.
+ ArrayRef<QualType> found() const { return types; }
+
+ // Look into the semantic form of initializer lists, which holds the backing
+ // array of a std::initializer_list, and into default member initializers.
+ // The array filler of an initializer list is not visited.
+ bool shouldVisitImplicitCode() const { return true; }
+
+ /// Records the type of each temporary requiring destruction whose lifetime
+ /// is extended by \c var.
+ bool VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *m) {
+ if (m->getExtendingDecl() != &var ||
+ m->getStorageDuration() != SD_Automatic || !visited.insert(m).second)
+ return true;
+ // Like pushTemporaryCleanup, look at the type of the whole temporary, not
+ // at the subobject that the reference binds to.
+ SmallVector<const Expr *, 2> commaLHSs;
+ SmallVector<SubobjectAdjustment, 2> adjustments;
+ const Expr *e =
+ m->getSubExpr()->skipRValueSubobjectAdjustments(commaLHSs,
adjustments);
+ if (e->getType().isDestructedType() != QualType::DK_none)
+ types.push_back(e->getType());
+ return true;
+ }
+};
+} // namespace
+
+static bool destructorMayThrow(QualType type) {
+ const CXXRecordDecl *rd =
+ type->getBaseElementTypeUnsafe()->getAsCXXRecordDecl();
+ if (!rd || rd->hasTrivialDestructor())
+ return false;
+ const auto *fpt =
rd->getDestructor()->getType()->castAs<FunctionProtoType>();
+ return isUnresolvedExceptionSpec(fpt->getExceptionSpecType()) ||
+ !fpt->isNothrow();
+}
+
+SmallVector<QualType, 2>
+CIRGenFunction::getLifetimeExtendedTemporariesWithDestructor(const VarDecl &d)
{
+ const Expr *init = d.getInit();
+ if (!init)
+ return {};
+ LifetimeExtendedTemporaryFinder finder(d);
+ finder.TraverseStmt(const_cast<Expr *>(init));
+ return SmallVector<QualType, 2>(finder.found());
+}
+
+void CIRGenFunction::emitLoopConditionVariable(
+ const VarDecl &d, CapturedCleanups &condCleanup,
+ ArrayRef<QualType> extendedTemporaries) {
// A condition variable always has automatic storage duration, so this
// mirrors the auto-var path of emitVarDecl/emitAutoVarDecl. Capture the
// lifetime-end cleanup pushed while emitting the alloca, but emit the
- // initializer with capturing disabled so its own cleanups get their normal
- // cir.cleanup.scope handling. The variable's destructor cleanup is captured
+ // initializer with capturing disabled so the cleanups of its temporaries get
+ // their normal cir.cleanup.scope handling. Only the cleanups of the
+ // temporaries that it lifetime-extends are captured, when its
+ // full-expression ends. The variable's destructor cleanup is captured
// separately after initialization.
assert(d.hasLocalStorage() && "loop condition variable is not local");
@@ -419,9 +484,27 @@ void CIRGenFunction::emitLoopConditionVariable(const
VarDecl &d,
// false while the initializer runs and set to true once construction
// completes. The flag is stored to on every iteration, so it also resets
// correctly across iterations.
+ //
+ // The temporaries that the initializer lifetime-extends are destroyed with
+ // the variable as well ([class.temporary]p6). Their cleanups are pushed when
+ // the initializer's full-expression ends, so capture them too and guard them
+ // with the same flag.
bool needsCleanup = d.needsDestruction(getContext()) != QualType::DK_none;
+ bool extendsTemporaries = !extendedTemporaries.empty();
+
+ // Destroy the variable first, then the temporaries in reverse order of
+ // construction. A throw would skip the remaining destructors because CFG
+ // flattening does not yet support EH paths in loop cleanup regions.
+ if (getLangOpts().Exceptions && extendsTemporaries &&
+ ((needsCleanup && destructorMayThrow(d.getType())) ||
+ (extendedTemporaries.size() > 1 &&
+ llvm::any_of(extendedTemporaries, destructorMayThrow))))
+ cgm.errorNYI(d.getSourceRange(),
+ "loop condition variable cleanup with a potentially-throwing "
+ "destructor before another destructor");
+
Address activeFlag = Address::invalid();
- if (needsCleanup) {
+ if (needsCleanup || extendsTemporaries) {
mlir::Location loc = getLoc(d.getSourceRange());
activeFlag = createTempAllocaWithoutCast(
builder.getBoolTy(), CharUnits::One(), loc, "cond.cleanup.isactive",
@@ -430,10 +513,31 @@ void CIRGenFunction::emitLoopConditionVariable(const
VarDecl &d,
builder.createFlagStore(loc, false, activeFlag.getPointer());
}
- emitAutoVarInit(emission);
+ EHScopeStack::stable_iterator initDepth = ehStack.stable_begin();
+ {
+ LifetimeExtensionCapture capture{&d, &condCleanup, activeFlag};
+ llvm::SaveAndRestore<const LifetimeExtensionCapture *> captureExtended(
+ lifetimeExtensionCapture, extendsTemporaries ? &capture : nullptr);
+ emitAutoVarInit(emission);
+ }
- if (needsCleanup) {
- // Construction has completed, so activate the destructor cleanup.
+ // Only lifetime-extended cleanups remain after initialization. If the AST
+ // walk found no extended temporaries, these cleanups are uncaptured and wrap
+ // cir.condition in cir.cleanup.scope (e.g. for initializer-list array
fillers).
+ if (!extendsTemporaries && ehStack.stable_begin() != initDepth) {
+ cgm.errorNYI(d.getSourceRange(),
+ "loop condition variable whose initializer leaves uncaptured "
+ "cleanups");
+ popCleanupBlocks(initDepth);
+ }
+#ifndef NDEBUG
+ for (auto it = ehStack.begin(), ie = ehStack.find(initDepth); it != ie; ++it)
+ assert(!cast<EHCleanupScope>(*it).getCleanupScopeOp() &&
+ "loop condition variable initializer left an uncaptured cleanup");
+#endif
+
+ if (activeFlag.isValid()) {
+ // Construction has completed, so activate the destructor cleanups.
mlir::Location loc = getLoc(d.getSourceRange());
builder.createFlagStore(loc, true, activeFlag.getPointer());
}
@@ -1222,10 +1326,9 @@ void CIRGenFunction::pushDestroyAndDeferDeactivation(
destroyer);
}
-void CIRGenFunction::pushLifetimeExtendedDestroy(CleanupKind cleanupKind,
- Address addr, QualType type,
- Destroyer *destroyer,
- bool useEHCleanupForArray) {
+void CIRGenFunction::pushLifetimeExtendedDestroy(
+ CleanupKind cleanupKind, Address addr, QualType type, Destroyer *destroyer,
+ bool useEHCleanupForArray, const ValueDecl *extendingDecl) {
if (isInConditionalBranch()) {
cgm.errorNYI("conditional lifetime-extended destroy");
return;
@@ -1239,10 +1342,22 @@ void
CIRGenFunction::pushLifetimeExtendedDestroy(CleanupKind cleanupKind,
assert(!cir::MissingFeatures::useEHCleanupForArray());
pushCleanupAfterFullExpr(cleanupKind, addr, type, destroyer);
+
+ // A temporary that a loop condition variable extends is destroyed with the
+ // variable at the end of each iteration, in the loop's cleanup region.
+ if (lifetimeExtensionCapture &&
+ extendingDecl == lifetimeExtensionCapture->extendingDecl) {
+ PendingCleanupEntry &entry = lifetimeExtendedCleanupStack.back();
+ entry.activeFlag = lifetimeExtensionCapture->activeFlag;
+ entry.capturedBy = lifetimeExtensionCapture->cleanups;
+ }
}
void CIRGenFunction::pushPendingCleanupToEHStack(
const PendingCleanupEntry &entry) {
+ std::optional<CapturedCleanups::CaptureScope> capture;
+ if (entry.capturedBy)
+ capture.emplace(*entry.capturedBy);
ehStack.pushCleanup<DestroyObject>(entry.kind, entry.addr, entry.type,
entry.destroyer);
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index 619ad771a97c9..bc1364823f514 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -2026,7 +2026,8 @@ static void pushTemporaryCleanup(CIRGenFunction &cgf,
case SD_Automatic:
cgf.pushLifetimeExtendedDestroy(
NormalAndEHCleanup, referenceTemporary, e->getType(),
- CIRGenFunction::destroyCXXObject, cgf.getLangOpts().Exceptions);
+ CIRGenFunction::destroyCXXObject, cgf.getLangOpts().Exceptions,
+ m->getExtendingDecl());
break;
case SD_Dynamic:
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h
b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index 6ca9f27748b28..32ce202b79279 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -129,6 +129,8 @@ class CIRGenFunction : public CIRGenTypeCache {
typedef void Destroyer(CIRGenFunction &cgf, Address addr, QualType ty);
+ class CapturedCleanups;
+
/// A cleanup whose destructor call is not emitted where the cleanup is
/// registered. Used by the lifetime-extended cleanup stack, whose entries
/// are later promoted onto the EH scope stack, and by the deferred
@@ -148,6 +150,9 @@ class CIRGenFunction : public CIRGenTypeCache {
QualType type;
Destroyer *destroyer;
Address activeFlag = Address::invalid();
+ /// If set, the cleanup is captured by this owner instead of opening a
+ /// cir.cleanup.scope when it is pushed onto the EH stack.
+ CapturedCleanups *capturedBy = nullptr;
};
llvm::SmallVector<PendingCleanupEntry> lifetimeExtendedCleanupStack;
@@ -200,6 +205,19 @@ class CIRGenFunction : public CIRGenTypeCache {
/// what closes those scopes.
cir::CleanupScopeOp currentFullExprCleanupScope = nullptr;
+ /// Set by emitLoopConditionVariable while it emits the initializer of a loop
+ /// condition variable that lifetime-extends temporaries with a destructor.
+ /// The cleanups of the temporaries that \c extendingDecl extends are
+ /// captured by \c cleanups for the loop's cleanup region and guarded by
+ /// \c activeFlag, instead of opening cir.cleanup.scope operations around the
+ /// cir.condition.
+ struct LifetimeExtensionCapture {
+ const clang::ValueDecl *extendingDecl;
+ CapturedCleanups *cleanups;
+ Address activeFlag;
+ };
+ const LifetimeExtensionCapture *lifetimeExtensionCapture = nullptr;
+
/// A cleanup that was pushed to the EH stack but whose deactivation is
/// deferred until the enclosing CleanupDeactivationScope exits. Used to
/// protect partially-constructed aggregates (e.g. lambda captures) so that
@@ -1730,7 +1748,8 @@ class CIRGenFunction : public CIRGenTypeCache {
void pushLifetimeExtendedDestroy(CleanupKind kind, Address addr,
QualType type, Destroyer *destroyer,
- bool useEHCleanupForArray);
+ bool useEHCleanupForArray,
+ const clang::ValueDecl *extendingDecl);
Destroyer *getDestroyer(clang::QualType::DestructionKind kind);
@@ -1907,8 +1926,17 @@ class CIRGenFunction : public CIRGenTypeCache {
/// Emit a loop's condition-variable declaration. This needs special handling
/// so that we can manage per-iteration cleanups for the loop condition.
+ /// \p extendedTemporaries are the types of the temporaries with a destructor
+ /// that its initializer lifetime-extends (see
+ /// getLifetimeExtendedTemporariesWithDestructor).
void emitLoopConditionVariable(const clang::VarDecl &d,
- CapturedCleanups &condCleanup);
+ CapturedCleanups &condCleanup,
+ llvm::ArrayRef<QualType> extendedTemporaries);
+
+ /// The types of the temporaries with a destructor that the initializer of
+ /// the variable \p d, which has automatic storage duration,
lifetime-extends.
+ static llvm::SmallVector<QualType, 2>
+ getLifetimeExtendedTemporariesWithDestructor(const clang::VarDecl &d);
/// Emit the initializer for an allocated variable. If this call is not
/// associated with the call to emitAutoVarAlloca (as the address of the
diff --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
index d06d32f5b33c0..94c2f70eb5046 100644
--- a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
@@ -1030,14 +1030,20 @@ mlir::LogicalResult CIRGenFunction::emitForStmt(const
ForStmt &s) {
checkIfLoopMustProgress(s.getCond(), CodeGenUtils::hasEmptyLoopBody(s));
// A condition variable's lifetime is a single iteration, so capture its
- // destructor and lifetime-end cleanups and emit them into the loop's
+ // destructor and lifetime-end cleanups, and those of the temporaries that
+ // its initializer lifetime-extends, and emit them into the loop's
// per-iteration cleanup region. This scope is constructed after the
// init-statement so the init-statement's cleanups are not captured.
const VarDecl *condVar = s.getConditionVariable();
+ SmallVector<QualType, 2> condVarExtendedTemporaries;
+ if (condVar)
+ condVarExtendedTemporaries =
+ getLifetimeExtendedTemporariesWithDestructor(*condVar);
bool needsCondCleanup =
condVar &&
(condVar->needsDestruction(getContext()) != QualType::DK_none ||
- shouldEmitLifetimeMarkersForAutoVar());
+ shouldEmitLifetimeMarkersForAutoVar() ||
+ !condVarExtendedTemporaries.empty());
CapturedCleanups loopCondScope(*this, needsCondCleanup);
auto condBuilder = [&](mlir::OpBuilder &b, mlir::Location loc) {
@@ -1047,7 +1053,8 @@ mlir::LogicalResult CIRGenFunction::emitForStmt(const
ForStmt &s) {
if (s.getCond()) {
// If the for statement declares a condition variable, emit that here.
if (condVar)
- emitLoopConditionVariable(*condVar, loopCondScope);
+ emitLoopConditionVariable(*condVar, loopCondScope,
+ condVarExtendedTemporaries);
// C99 6.8.5p2/p4: The first substatement is executed if the
// expression compares unequal to 0. The condition must be a
// scalar type.
@@ -1165,13 +1172,19 @@ mlir::LogicalResult CIRGenFunction::emitWhileStmt(const
WhileStmt &s) {
checkIfLoopMustProgress(s.getCond(), CodeGenUtils::hasEmptyLoopBody(s));
// A condition variable's lifetime is a single iteration, so capture its
- // destructor and lifetime-end cleanups and emit them into the loop's
+ // destructor and lifetime-end cleanups, and those of the temporaries that
+ // its initializer lifetime-extends, and emit them into the loop's
// per-iteration cleanup region.
const VarDecl *condVar = s.getConditionVariable();
+ SmallVector<QualType, 2> condVarExtendedTemporaries;
+ if (condVar)
+ condVarExtendedTemporaries =
+ getLifetimeExtendedTemporariesWithDestructor(*condVar);
bool needsCondCleanup =
condVar &&
(condVar->needsDestruction(getContext()) != QualType::DK_none ||
- shouldEmitLifetimeMarkersForAutoVar());
+ shouldEmitLifetimeMarkersForAutoVar() ||
+ !condVarExtendedTemporaries.empty());
CapturedCleanups loopCondScope(*this, needsCondCleanup);
auto condBuilder = [&](mlir::OpBuilder &b, mlir::Location loc) {
@@ -1179,7 +1192,8 @@ mlir::LogicalResult CIRGenFunction::emitWhileStmt(const
WhileStmt &s) {
assert(!cir::MissingFeatures::emitCondLikelihoodViaExpectIntrinsic());
// If the while statement declares a condition variable, emit that here.
if (condVar)
- emitLoopConditionVariable(*condVar, loopCondScope);
+ emitLoopConditionVariable(*condVar, loopCondScope,
+ condVarExtendedTemporaries);
// C99 6.8.5p2/p4: The first substatement is executed if the
// expression compares unequal to 0. The condition must be a
// scalar type.
diff --git a/clang/test/CIR/CodeGen/lifetime-marker.cpp
b/clang/test/CIR/CodeGen/lifetime-marker.cpp
index 12ac98b3d5343..b0615cd1831c5 100644
--- a/clang/test/CIR/CodeGen/lifetime-marker.cpp
+++ b/clang/test/CIR/CodeGen/lifetime-marker.cpp
@@ -1,13 +1,13 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O2 -fclangir -emit-cir %s
-o %t.cir
-// RUN: FileCheck --input-file=%t.cir %s --check-prefix=CIR
+// RUN: FileCheck --input-file=%t.cir %s
--check-prefixes=CIR,EXT-CIR,EXT-CIR-NOEH
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O2 -fclangir -emit-llvm
-disable-llvm-passes %s -o %t.ll
-// RUN: FileCheck --input-file=%t.ll %s --check-prefix=LLVM
+// RUN: FileCheck --input-file=%t.ll %s
--check-prefixes=LLVM,EXT-LLVM,EXT-LLVM-NOEH
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o
%t-o0.cir
// RUN: FileCheck --input-file=%t-o0.cir %s --implicit-check-not "cir.lifetime"
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O1 -fcxx-exceptions
-fexceptions -fclangir -emit-cir %s -o %t-eh.cir
-// RUN: FileCheck --input-file=%t-eh.cir %s --check-prefix=CIR-EH
+// RUN: FileCheck --input-file=%t-eh.cir %s
--check-prefixes=CIR-EH,EXT-CIR,EXT-CIR-EH
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -O1 -fcxx-exceptions
-fexceptions -fclangir -emit-llvm -disable-llvm-passes %s -o %t-eh.ll
-// RUN: FileCheck --input-file=%t-eh.ll %s --check-prefixes=LLVM-EH
+// RUN: FileCheck --input-file=%t-eh.ll %s
--check-prefixes=LLVM-EH,EXT-LLVM,EXT-LLVM-EH
void use(int);
@@ -275,6 +275,79 @@ void while_record_condvar() {
// LLVM-EH: call void @_ZN8LoopCondD1Ev(ptr {{.*}} %[[C]])
// LLVM-EH: call void @llvm.lifetime.end.p0(ptr %[[C]])
+LoopCond makeLoopCond();
+
+// Destroy the extended temporary before ending the reference's lifetime.
+// CIR does not emit lifetime markers for the tempora...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/231469
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
