llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clangir
Author: Jiahao Guo (E00N777)
<details>
<summary>Changes</summary>
### summary
Emit cir.lifetime.start/end for alloca-backed temporaries materialized by
`MaterializeTemporaryExpr`
- Full-expression temporaries end their lifetime at the end of the
full-expression. Inside a conditional branch, the end is deferred to the
enclosing full-expression and guarded by an active flag.
- Lifetime-extended temporaries end their lifetime with the enclosing scope,
via the lifetime-extended cleanup stack. PendingCleanupEntry now also
represents CallLifetimeEnd (LifetimeMarker kind).
- Conditional branches keep lifetime-extended entries pending until the
enclosing full-expression ends (forceCleanup's deferLifetimeExtended).
I used Claude to review this PR, and I thought these might be useful:
<details>
Trivially destructible full-expression temporaries in a conditional branch are
handled differently from classic CodeGen.
Classic CodeGen hoists the temporary's lifetime.start to before the
conditional, so both markers are unconditional and no flag is needed.CIR emits
lifetime.start inside the branch and guards lifetime.end with an active flag
(`cir.if %flag { cir.lifetime.end }`) on both the normal and EH paths. The
result is correct, but it adds a flag alloca, flag stores, and extra
lifetime.end calls.
</details>
Assisted by : Claude opus5.5
---
Patch is 38.29 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/236102.diff
9 Files Affected:
- (modified) clang/lib/CIR/CodeGen/CIRGenCleanup.cpp (+48-3)
- (modified) clang/lib/CIR/CodeGen/CIRGenCleanup.h (+10)
- (modified) clang/lib/CIR/CodeGen/CIRGenDecl.cpp (+35-32)
- (modified) clang/lib/CIR/CodeGen/CIRGenExpr.cpp (+18-2)
- (modified) clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp (+1-1)
- (modified) clang/lib/CIR/CodeGen/CIRGenFunction.cpp (+1-1)
- (modified) clang/lib/CIR/CodeGen/CIRGenFunction.h (+52-22)
- (modified) clang/lib/CIR/CodeGen/CIRGenStmt.cpp (+10-12)
- (modified) clang/test/CIR/CodeGen/lifetime-marker.cpp (+211-12)
``````````diff
diff --git a/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp
b/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp
index 98fb9502ef0169..fd23efc4d12a7a 100644
--- a/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp
@@ -25,6 +25,52 @@
using namespace clang;
using namespace clang::CIRGen;
+void CallLifetimeEnd::emit(CIRGenFunction &cgf, Flags flags) {
+ cgf.emitLifetimeEndOp(addr.getLoc(), addr);
+}
+
+void CIRGenFunction::pushFullExprLifetimeEnd(Address addr) {
+ assert(addr.getDefiningOp<cir::AllocaOp>() &&
+ "lifetime cleanup requires the raw alloca address");
+ if (!isInConditionalBranch()) {
+ ehStack.pushCleanup<CallLifetimeEnd>(NormalEHLifetimeMarker,
+ addr.getPointer());
+ return;
+ }
+
+ Address activeFlag = createCleanupActiveFlag();
+ deferredConditionalCleanupStack.push_back(
+ {NormalEHLifetimeMarker, addr, QualType(), nullptr, activeFlag});
+}
+
+void CIRGenFunction::pushLifetimeEndAfterFullExpr(
+ Address addr, const ValueDecl *extendingDecl) {
+ assert(addr.getDefiningOp<cir::AllocaOp>() &&
+ "lifetime cleanup requires the raw alloca address");
+ Address activeFlag =
+ isInConditionalBranch() ? createCleanupActiveFlag() : Address::invalid();
+ PendingCleanupEntry entry{NormalEHLifetimeMarker, addr, QualType(), nullptr,
+ activeFlag};
+ // A loop condition variable's temporary ends its lifetime in the loop's
+ // cleanup region. Unlike its destructor, this is not guarded by the
+ // variable's active flag.
+ if (lifetimeExtensionCapture &&
+ lifetimeExtensionCapture->extendingDecl == extendingDecl)
+ entry.capturedBy = lifetimeExtensionCapture->cleanups;
+ lifetimeExtendedCleanupStack.push_back(entry);
+}
+
+void CIRGenFunction::emitPendingCleanup(const PendingCleanupEntry &entry) {
+ if (entry.kind & LifetimeMarker) {
+ assert(entry.addr.getDefiningOp<cir::AllocaOp>() &&
+ "lifetime cleanup requires the raw alloca address");
+ CallLifetimeEnd(entry.addr.getPointer())
+ .emit(*this, EHScopeStack::Cleanup::Flags());
+ return;
+ }
+ emitDestroy(entry.addr, entry.type, entry.destroyer);
+}
+
namespace {
/// Return true if the expression tree contains a construct that causes
cleanups
/// to be deferred via pushFullExprCleanup.
@@ -274,12 +320,11 @@ static void closeConditionalCleanupScope(
cir::IfOp::create(builder, scope.getLoc(), flag,
/*withElseRegion=*/false,
[&](mlir::OpBuilder &b, mlir::Location loc) {
- cgf.emitDestroy(entry.addr, entry.type,
- entry.destroyer);
+ cgf.emitPendingCleanup(entry);
builder.createYield(loc);
});
} else {
- cgf.emitDestroy(entry.addr, entry.type, entry.destroyer);
+ cgf.emitPendingCleanup(entry);
}
}
builder.createYield(scope.getLoc());
diff --git a/clang/lib/CIR/CodeGen/CIRGenCleanup.h
b/clang/lib/CIR/CodeGen/CIRGenCleanup.h
index 0f761b69c02d1d..1b4b2f202553ca 100644
--- a/clang/lib/CIR/CodeGen/CIRGenCleanup.h
+++ b/clang/lib/CIR/CodeGen/CIRGenCleanup.h
@@ -263,5 +263,15 @@ const EHPersonality &getEHPersonality(CIRGenModule &cgm,
/// emitted.
const EHPersonality &getEHPersonality(CIRGenFunction &cgf);
+struct CallLifetimeEnd final : EHScopeStack::Cleanup {
+ // The raw alloca pointer (in the alloca address space). Mirrors classic
+ // CodeGen's CallLifetimeEnd, which stores the llvm::Value pointer rather
+ // than an Address.
+ mlir::Value addr;
+ CallLifetimeEnd(mlir::Value addr) : addr(addr) {}
+ bool isRedundantBeforeReturn() override { return true; }
+ void emit(CIRGenFunction &cgf, Flags flags) override;
+};
+
} // namespace clang::CIRGen
#endif // CLANG_LIB_CIR_CODEGEN_CIRGENCLEANUP_H
diff --git a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
index 17d832046363fb..fe54f0d02232ed 100644
--- a/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenDecl.cpp
@@ -32,18 +32,6 @@
using namespace clang;
using namespace clang::CIRGen;
-struct CallLifetimeEnd final : EHScopeStack::Cleanup {
- // The raw alloca pointer (in the alloca address space). Mirrors classic
- // CodeGen's CallLifetimeEnd, which stores the llvm::Value pointer rather
- // than an Address.
- mlir::Value addr;
- CallLifetimeEnd(mlir::Value addr) : addr(addr) {}
- bool isRedundantBeforeReturn() override { return true; }
- void emit(CIRGenFunction &cgf, Flags flags) override {
- cgf.emitLifetimeEndOp(addr.getLoc(), addr);
- }
-};
-
CIRGenFunction::AutoVarEmission
CIRGenFunction::emitAutoVarAlloca(const VarDecl &d,
mlir::OpBuilder::InsertPoint ip) {
@@ -390,32 +378,37 @@ void CIRGenFunction::emitAutoVarDecl(const VarDecl &d) {
}
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.
+/// Finds the temporaries 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;
+ bool any = false;
public:
explicit LifetimeExtendedTemporaryFinder(const VarDecl &var) : var(var) {}
- /// The types of the temporaries that were found.
+ /// The types of the found temporaries that have a destructor.
ArrayRef<QualType> found() const { return types; }
+ /// Whether any temporary was found, including trivially destructible ones.
+ bool foundAny() const { return any; }
+
// 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.
+ /// Records each temporary whose lifetime is extended by \c var, and the
+ /// type of those requiring destruction.
bool VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *m) {
if (m->getExtendingDecl() != &var ||
m->getStorageDuration() != SD_Automatic || !visited.insert(m).second)
return true;
+ any = 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;
@@ -439,19 +432,19 @@ static bool destructorMayThrow(QualType type) {
!fpt->isNothrow();
}
-SmallVector<QualType, 2>
-CIRGenFunction::getLifetimeExtendedTemporariesWithDestructor(const VarDecl &d)
{
+CIRGenFunction::ExtendedTemporaries
+CIRGenFunction::getLifetimeExtendedTemporaries(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());
+ return {SmallVector<QualType, 2>(finder.found()), finder.foundAny()};
}
void CIRGenFunction::emitLoopConditionVariable(
const VarDecl &d, CapturedCleanups &condCleanup,
- ArrayRef<QualType> extendedTemporaries) {
+ const ExtendedTemporaries &temps) {
// 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
@@ -488,17 +481,20 @@ void CIRGenFunction::emitLoopConditionVariable(
// 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.
+ // with the same flag. Their lifetime-end cleanups are captured unguarded, as
+ // their lifetimes start before the initializer runs.
bool needsCleanup = d.needsDestruction(getContext()) != QualType::DK_none;
- bool extendsTemporaries = !extendedTemporaries.empty();
+ bool extendsTemporaries = !temps.withDestructor.empty();
+ bool capturesExtendedCleanups =
+ extendsTemporaries || extendedTemporariesNeedLifetimeEnd(temps);
// 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))))
+ (temps.withDestructor.size() > 1 &&
+ llvm::any_of(temps.withDestructor, destructorMayThrow))))
cgm.errorNYI(d.getSourceRange(),
"loop condition variable cleanup with a potentially-throwing "
"destructor before another destructor");
@@ -517,15 +513,16 @@ void CIRGenFunction::emitLoopConditionVariable(
{
LifetimeExtensionCapture capture{&d, &condCleanup, activeFlag};
llvm::SaveAndRestore<const LifetimeExtensionCapture *> captureExtended(
- lifetimeExtensionCapture, extendsTemporaries ? &capture : nullptr);
+ lifetimeExtensionCapture,
+ capturesExtendedCleanups ? &capture : nullptr);
emitAutoVarInit(emission);
}
- // Only lifetime-extended cleanups remain after initialization. If the AST
- // walk found no extended temporaries, these cleanups are uncaptured and wrap
+ // Only lifetime-extended cleanups remain after initialization. If none were
+ // expected to be captured, 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) {
+ if (!capturesExtendedCleanups && ehStack.stable_begin() != initDepth) {
cgm.errorNYI(d.getSourceRange(),
"loop condition variable whose initializer leaves uncaptured "
"cleanups");
@@ -1360,8 +1357,14 @@ void CIRGenFunction::pushPendingCleanupToEHStack(
std::optional<CapturedCleanups::CaptureScope> capture;
if (entry.capturedBy)
capture.emplace(*entry.capturedBy);
- ehStack.pushCleanup<DestroyObject>(entry.kind, entry.addr, entry.type,
- entry.destroyer);
+ if (entry.kind & LifetimeMarker) {
+ assert(entry.addr.getDefiningOp<cir::AllocaOp>() &&
+ "lifetime cleanup requires the raw alloca address");
+ ehStack.pushCleanup<CallLifetimeEnd>(entry.kind, entry.addr.getPointer());
+ } else {
+ ehStack.pushCleanup<DestroyObject>(entry.kind, entry.addr, entry.type,
+ entry.destroyer);
+ }
if (entry.activeFlag.isValid()) {
EHCleanupScope &scope = cast<EHCleanupScope>(*ehStack.begin());
diff --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index 321ebaaa34517c..d41b2fa054d498 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//
#include "Address.h"
+#include "CIRGenCleanup.h"
#include "CIRGenConstantEmitter.h"
#include "CIRGenFunction.h"
#include "CIRGenModule.h"
@@ -2119,7 +2120,22 @@ LValue CIRGenFunction::emitMaterializeTemporaryExpr(
emitAnyExprToMem(e, object, Qualifiers(), /*isInitializer=*/true);
}
} else {
- assert(!cir::MissingFeatures::emitLifetimeMarkers());
+ mlir::Location loc = getLoc(m->getSourceRange());
+ if (cir::AllocaOp alloca = object.getUnderlyingAllocaOp()) {
+ if (emitLifetimeStartOp(loc, alloca.getResult())) {
+ Address lifetimeAddr(alloca.getResult(), object.getAlignment());
+ switch (m->getStorageDuration()) {
+ case SD_FullExpression:
+ pushFullExprLifetimeEnd(lifetimeAddr);
+ break;
+ case SD_Automatic:
+ pushLifetimeEndAfterFullExpr(lifetimeAddr, m->getExtendingDecl());
+ break;
+ default:
+ llvm_unreachable("alloca-backed temporary has unexpected duration");
+ }
+ }
+ }
emitAnyExprToMem(e, object, Qualifiers(), /*isInitializer=*/true);
}
pushTemporaryCleanup(*this, m, e, object);
@@ -3263,7 +3279,7 @@ CIRGenFunction::emitConditionalBlocks(const
AbstractConditionalOperator *e,
resultLV = branchGenFunc(*this, expr);
resultPtr = resultLV ? resultLV->getPointer() : mlir::Value();
eval.endEvaluation();
- branchCleanups.forceCleanup({&resultPtr});
+ branchCleanups.forceCleanup({&resultPtr},
/*deferLifetimeExtended=*/true);
}
// A branch that produced no result is a throw-expression; its region is
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index c27f7ef7417e5e..4b282795b64a7f 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -3360,7 +3360,7 @@ mlir::Value
ScalarExprEmitter::VisitAbstractConditionalOperator(
eval.beginEvaluation();
branch = Visit(expr);
eval.endEvaluation();
- branchCleanups.forceCleanup({&branch});
+ branchCleanups.forceCleanup({&branch}, /*deferLifetimeExtended=*/true);
}
if (branch)
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
index 1fa57c4087da2c..65b2e00eb3bee0 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.cpp
@@ -309,7 +309,7 @@ void CIRGenFunction::LexicalScope::cleanup() {
// blocks or branches are needed.
if (performCleanup) {
assert(!cir::MissingFeatures::generateDebugInfo());
- forceCleanup();
+ forceCleanup({}, /*deferLifetimeExtended=*/isTernary());
}
mlir::Block *curBlock = builder.getBlock();
diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h
b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index 90da139c0f19c7..2e552abd8927c9 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -134,19 +134,18 @@ class CIRGenFunction : public CIRGenTypeCache {
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
+ /// A cleanup that is not emitted where it is registered. Used by the
+ /// lifetime-extended cleanup stack, whose entries are later promoted onto
+ /// the EH scope stack, and by the deferred
/// conditional cleanup stack, whose entries are emitted directly into a
/// conditional scope's cleanup region.
///
/// A valid \c activeFlag means the cleanup is conditional. The flag tracks
- /// at run time whether the object has been constructed, and guards the
- /// destructor call.
+ /// at run time whether the branch activated the cleanup.
///
- /// Currently only DestroyObject cleanups use this. When other cleanup types
- /// are needed (e.g., CallLifetimeEnd), this struct can be extended with a
- /// std::variant of cleanup data types.
+ /// LifetimeMarker entries represent CallLifetimeEnd: addr is the raw alloca
+ /// address, and type and destroyer are unused. Other entries represent
+ /// DestroyObject and carry its type and destroyer.
struct PendingCleanupEntry {
CleanupKind kind;
Address addr;
@@ -1315,6 +1314,18 @@ class CIRGenFunction : public CIRGenTypeCache {
/// Defined in CIRGenDecl.cpp where the concrete cleanup types are visible.
void pushPendingCleanupToEHStack(const PendingCleanupEntry &entry);
+ /// Emit a deferred cleanup at the current insertion point.
+ void emitPendingCleanup(const PendingCleanupEntry &entry);
+
+ /// Register a storage lifetime end at the end of the current
full-expression,
+ /// deferring it with an active flag when inside a conditional branch.
+ void pushFullExprLifetimeEnd(Address addr);
+
+ /// Register a storage lifetime end for the enclosing scope, after the
current
+ /// full-expression has finished.
+ void pushLifetimeEndAfterFullExpr(Address addr,
+ const ValueDecl *extendingDecl);
+
/// Push a cleanup to be run at the end of the current full-expression. Safe
/// against the possibility that we're currently inside a
/// conditionally-evaluated expression.
@@ -1396,7 +1407,10 @@ class CIRGenFunction : public CIRGenTypeCache {
/// Force the emission of cleanups now, instead of waiting
/// until this object is destroyed.
- void forceCleanup(ArrayRef<mlir::Value *> valuesToReload = {}) {
+ /// Conditional arms leave lifetime-extended entries pending until the
+ /// enclosing full expression ends.
+ void forceCleanup(ArrayRef<mlir::Value *> valuesToReload = {},
+ bool deferLifetimeExtended = false) {
assert(performCleanup && "Already forced cleanup");
cgf.didCallStackSave = oldDidCallStackSave;
@@ -1442,10 +1456,15 @@ class CIRGenFunction : public CIRGenTypeCache {
// by either forceDeactivate's or popCleanupBlocks's pops (both only
// pop cir.cleanup.scope ops); they already dominate the post-scope
// insertion point on their own.
- if (didSpillAny) {
+ ArrayRef<mlir::Value *> valuesToPop =
+ didSpillAny ? ArrayRef<mlir::Value *>() : valuesToReload;
+ if (deferLifetimeExtended)
+ cgf.popCleanupBlocks(cleanupStackDepth, valuesToPop);
+ else
cgf.popCleanupBlocks(cleanupStackDepth,
- lifetimeExtendedCleanupStackSize);
+ lifetimeExtendedCleanupStackSize, valuesToPop);
+ if (didSpillAny) {
// Reload the spilled values now that all cleanup popping (and
// promotion of any lifetime-extended cleanups onto the EH stack) is
// done.
@@ -1454,9 +1473,6 @@ class CIRGenFunction : public CIRGenTypeCache {
continue;
*valPtr = cgf.builder.createLoad(valPtr->getLoc(), addr);
}
- } else {
- cgf.popCleanupBlocks(cleanupStackDepth,
- lifetimeExtendedCleanupStackSize, valuesToReload);
}
performCleanup = false;
@@ -1942,19 +1958,33 @@ class CIRGenFunction : public CIRGenTypeCache {
void emitAutoVarCleanups(const AutoVarEmission &emission);
+ /// The temporaries that a variable's initializer lifetime-extends.
+ struct ExtendedTemporaries {
+ /// The types of those with a destructor.
+ SmallVector<QualType, 2> withDestructor;
+ /// Whether any temporary is extended, including trivially destructible
+ /// ones.
+ bool any = false;
+ };
+
/// 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).
+ /// \p temps are the temporaries that its initializer lifetime-extends (see
+ /// getLifetimeExtendedTemporaries).
void emitLoopConditionVariable(const clang::VarDecl &d,
CapturedCleanups &condCleanup,
- llvm::ArrayRef<QualType> extendedTemporaries);
+ const ExtendedTemporaries &temps);
+
+ /// Whether the extended temporaries \p t get lifetime-end cleanups. This
+ /// must match the check in emitLifetimeStartOp.
+ bool extendedTemporariesNeedLifetimeEnd(const ExtendedTemporaries &t) const {
+ return shouldEmitLifetimeMarkers && t.any;
+ }
- /// The types of the temporaries with a dest...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/236102
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
