https://github.com/andykaylor created https://github.com/llvm/llvm-project/pull/152589
Support for normal cleanups was introduced with a simplified implementation compared to what's in the incubator (which corresponds closely to the classic codegen implementation). This change introduces more of the infrastructure that will later be needed to handle non-trivial cleanup cases, including exception handling. >From 8db87db943e8c760fc10abe4ddfb8eb15f5ee245 Mon Sep 17 00:00:00 2001 From: Andy Kaylor <akay...@nvidia.com> Date: Wed, 6 Aug 2025 14:22:27 -0700 Subject: [PATCH] [CIR] Introduce more cleanup infrastructure Support for normal cleanups was introduced with a simplified implementation compared to what's in the incubator (which corresponds closely to the classic codegen implementation). This change introduces more of the infrastructure that will later be needed to handle non-trivial cleanup cases, including exception handling. --- clang/include/clang/CIR/MissingFeatures.h | 2 + clang/lib/CIR/CodeGen/CIRGenCleanup.cpp | 88 ++++++++++++----- clang/lib/CIR/CodeGen/CIRGenCleanup.h | 113 +++++++++++++++++++++- clang/lib/CIR/CodeGen/CIRGenFunction.h | 9 ++ 4 files changed, 186 insertions(+), 26 deletions(-) diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h index fcc8ce7caf111..926f2c57e5968 100644 --- a/clang/include/clang/CIR/MissingFeatures.h +++ b/clang/include/clang/CIR/MissingFeatures.h @@ -200,6 +200,8 @@ struct MissingFeatures { static bool deferredCXXGlobalInit() { return false; } static bool ehCleanupFlags() { return false; } static bool ehCleanupScope() { return false; } + static bool ehCleanupScopeRequiresEHCleanup() { return false; } + static bool ehCleanupBranchFixups() { return false; } static bool ehstackBranches() { return false; } static bool emitCheckedInBoundsGEP() { return false; } static bool emitCondLikelihoodViaExpectIntrinsic() { return false; } diff --git a/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp b/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp index b8663eb951d05..865324cb8d44f 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenCleanup.cpp @@ -70,21 +70,32 @@ void EHScopeStack::deallocate(size_t size) { } void *EHScopeStack::pushCleanup(CleanupKind kind, size_t size) { - char *buffer = allocate(size); + char *buffer = allocate(EHCleanupScope::getSizeForCleanupSize(size)); - // When the full implementation is upstreamed, this will allocate - // extra memory for and construct a wrapper object that is used to - // manage the cleanup generation. - assert(!cir::MissingFeatures::ehCleanupScope()); + EHCleanupScope *scope = new (buffer) EHCleanupScope(size); - return buffer; + return scope->getCleanupBuffer(); } -static mlir::Block *getCurCleanupBlock(CIRGenFunction &cgf) { - mlir::OpBuilder::InsertionGuard guard(cgf.getBuilder()); - mlir::Block *cleanup = - cgf.curLexScope->getOrCreateCleanupBlock(cgf.getBuilder()); - return cleanup; +void EHScopeStack::popCleanup() { + assert(!empty() && "popping exception stack when not empty"); + + assert(isa<EHCleanupScope>(*begin())); + EHCleanupScope &cleanup = cast<EHCleanupScope>(*begin()); + deallocate(cleanup.getAllocatedSize()); + + // Destroy the cleanup. + cleanup.destroy(); + + assert(!cir::MissingFeatures::ehCleanupBranchFixups()); +} + +static void emitCleanup(CIRGenFunction &cgf, EHScopeStack::Cleanup *cleanup) { + // Ask the cleanup to emit itself. + assert(cgf.haveInsertPoint() && "expected insertion point"); + assert(!cir::MissingFeatures::ehCleanupFlags()); + cleanup->emit(cgf); + assert(cgf.haveInsertPoint() && "cleanup ended with no insertion point?"); } /// Pops a cleanup block. If the block includes a normal cleanup, the @@ -92,23 +103,56 @@ static mlir::Block *getCurCleanupBlock(CIRGenFunction &cgf) { /// any branch fixups on the cleanup. void CIRGenFunction::popCleanupBlock() { assert(!ehStack.empty() && "cleanup stack is empty!"); + assert(isa<EHCleanupScope>(*ehStack.begin()) && "top not a cleanup!"); + EHCleanupScope &scope = cast<EHCleanupScope>(*ehStack.begin()); - // The memory for the cleanup continues to be owned by the EHScopeStack - // allocator, so we just destroy the object rather than attempting to - // free it. - EHScopeStack::Cleanup &cleanup = *ehStack.begin(); + // Remember activation information. + bool isActive = scope.isActive(); - // The eventual implementation here will use the EHCleanupScope helper class. - assert(!cir::MissingFeatures::ehCleanupScope()); + assert(!cir::MissingFeatures::ehCleanupBranchFixups()); - mlir::OpBuilder::InsertionGuard guard(builder); + // - whether there's a fallthrough + mlir::Block *fallthroughSource = builder.getInsertionBlock(); + bool hasFallthrough = fallthroughSource != nullptr && isActive; + + bool requiresNormalCleanup = scope.isNormalCleanup() && hasFallthrough; + + // If we don't need the cleanup at all, we're done. + assert(!cir::MissingFeatures::ehCleanupScopeRequiresEHCleanup()); + if (!requiresNormalCleanup) { + ehStack.popCleanup(); + return; + } + + // Copy the cleanup emission data out. This uses either a stack + // array or malloc'd memory, depending on the size, which is + // behavior that SmallVector would provide, if we could use it + // here. Unfortunately, if you ask for a SmallVector<char>, the + // alignment isn't sufficient. + auto *cleanupSource = reinterpret_cast<char *>(scope.getCleanupBuffer()); + alignas(EHScopeStack::ScopeStackAlignment) char + cleanupBufferStack[8 * sizeof(void *)]; + std::unique_ptr<char[]> cleanupBufferHeap; + size_t cleanupSize = scope.getCleanupSize(); + EHScopeStack::Cleanup *cleanup; + + // This is necessary because we are going to deallocate the cleanup + // (in popCleanup) before we emit it. + if (cleanupSize <= sizeof(cleanupBufferStack)) { + memcpy(cleanupBufferStack, cleanupSource, cleanupSize); + cleanup = reinterpret_cast<EHScopeStack::Cleanup *>(cleanupBufferStack); + } else { + cleanupBufferHeap.reset(new char[cleanupSize]); + memcpy(cleanupBufferHeap.get(), cleanupSource, cleanupSize); + cleanup = + reinterpret_cast<EHScopeStack::Cleanup *>(cleanupBufferHeap.get()); + } assert(!cir::MissingFeatures::ehCleanupFlags()); - mlir::Block *cleanupEntry = getCurCleanupBlock(*this); - builder.setInsertionPointToEnd(cleanupEntry); - cleanup.emit(*this); - ehStack.deallocate(cleanup.getSize()); + ehStack.popCleanup(); + scope.markEmitted(); + emitCleanup(*this, cleanup); } /// Pops cleanup blocks until the given savepoint is reached. diff --git a/clang/lib/CIR/CodeGen/CIRGenCleanup.h b/clang/lib/CIR/CodeGen/CIRGenCleanup.h index 7361c8c00891b..0de228993a630 100644 --- a/clang/lib/CIR/CodeGen/CIRGenCleanup.h +++ b/clang/lib/CIR/CodeGen/CIRGenCleanup.h @@ -14,10 +14,117 @@ #ifndef CLANG_LIB_CIR_CODEGEN_CIRGENCLEANUP_H #define CLANG_LIB_CIR_CODEGEN_CIRGENCLEANUP_H +#include "Address.h" #include "EHScopeStack.h" +#include "mlir/IR/Value.h" namespace clang::CIRGen { +/// A protected scope for zero-cost EH handling. +class EHScope { + class CommonBitFields { + friend class EHScope; + unsigned kind : 3; + }; + enum { NumCommonBits = 3 }; + +protected: + class CleanupBitFields { + friend class EHCleanupScope; + unsigned : NumCommonBits; + + /// Whether this cleanup needs to be run along normal edges. + unsigned isNormalCleanup : 1; + + /// Whether this cleanup needs to be run along exception edges. + unsigned isEHCleanup : 1; + + /// Whether this cleanup is currently active. + unsigned isActive : 1; + + /// Whether this cleanup is a lifetime marker + unsigned isLifetimeMarker : 1; + + /// Whether the normal cleanup should test the activation flag. + unsigned testFlagInNormalCleanup : 1; + + /// Whether the EH cleanup should test the activation flag. + unsigned testFlagInEHCleanup : 1; + + /// The amount of extra storage needed by the Cleanup. + /// Always a multiple of the scope-stack alignment. + unsigned cleanupSize : 12; + }; + + union { + CommonBitFields commonBits; + CleanupBitFields cleanupBits; + }; + +public: + enum Kind { Cleanup, Catch, Terminate, Filter }; + + EHScope(Kind kind) { commonBits.kind = kind; } + + Kind getKind() const { return static_cast<Kind>(commonBits.kind); } +}; + +/// A cleanup scope which generates the cleanup blocks lazily. +class alignas(8) EHCleanupScope : public EHScope { +public: + /// Gets the size required for a lazy cleanup scope with the given + /// cleanup-data requirements. + static size_t getSizeForCleanupSize(size_t size) { + return sizeof(EHCleanupScope) + size; + } + + size_t getAllocatedSize() const { + return sizeof(EHCleanupScope) + cleanupBits.cleanupSize; + } + + EHCleanupScope(unsigned cleanupSize) : EHScope(EHScope::Cleanup) { + // TODO(cir): When exception handling is upstreamed, isNormalCleanup and + // isEHCleanup will be arguments to the constructor. + cleanupBits.isNormalCleanup = true; + cleanupBits.isEHCleanup = false; + cleanupBits.isActive = true; + cleanupBits.isLifetimeMarker = false; + cleanupBits.testFlagInNormalCleanup = false; + cleanupBits.testFlagInEHCleanup = false; + cleanupBits.cleanupSize = cleanupSize; + + assert(cleanupBits.cleanupSize == cleanupSize && "cleanup size overflow"); + } + + void destroy() {} + // Objects of EHCleanupScope are not destructed. Use destroy(). + ~EHCleanupScope() = delete; + + bool isNormalCleanup() const { return cleanupBits.isNormalCleanup; } + + bool isActive() const { return cleanupBits.isActive; } + + size_t getCleanupSize() const { return cleanupBits.cleanupSize; } + void *getCleanupBuffer() { return this + 1; } + + EHScopeStack::Cleanup *getCleanup() { + return reinterpret_cast<EHScopeStack::Cleanup *>(getCleanupBuffer()); + } + + static bool classof(const EHScope *scope) { + return (scope->getKind() == Cleanup); + } + + void markEmitted() {} +}; +// NOTE: There can be additional data classes tacked on after an EHCleanupScope. +// It is asserted (in EHScopeStack::pushCleanup*) that they don't require +// greater alignment than ScopeStackAlignment. So, EHCleanupScope must have +// alignment equal to that -- not more (would be misaligned by the stack +// allocator), and not less (would break the appended classes). +static_assert(alignof(EHCleanupScope) == EHScopeStack::ScopeStackAlignment, + "EHCleanupScope expected alignment"); + /// A non-stable pointer into the scope stack. class EHScopeStack::iterator { char *ptr = nullptr; @@ -28,11 +135,9 @@ class EHScopeStack::iterator { public: iterator() = default; - EHScopeStack::Cleanup *get() const { - return reinterpret_cast<EHScopeStack::Cleanup *>(ptr); - } + EHScope *get() const { return reinterpret_cast<EHScope *>(ptr); } - EHScopeStack::Cleanup &operator*() const { return *get(); } + EHScope &operator*() const { return *get(); } }; inline EHScopeStack::iterator EHScopeStack::begin() const { diff --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h index bdbc77c0a7c4a..ccb9fb1cc7840 100644 --- a/clang/lib/CIR/CodeGen/CIRGenFunction.h +++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h @@ -337,6 +337,15 @@ class CIRGenFunction : public CIRGenTypeCache { const clang::LangOptions &getLangOpts() const { return cgm.getLangOpts(); } + /// True if an insertion point is defined. If not, this indicates that the + /// current code being emitted is unreachable. + /// FIXME(cir): we need to inspect this and perhaps use a cleaner mechanism + /// since we don't yet force null insertion point to designate behavior (like + /// LLVM's codegen does) and we probably shouldn't. + bool haveInsertPoint() const { + return builder.getInsertionBlock() != nullptr; + } + // Wrapper for function prototype sources. Wraps either a FunctionProtoType or // an ObjCMethodDecl. struct PrototypeWrapper { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits