-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[CIR] Introduce more cleanup infrastructure #152589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,45 +70,89 @@ 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 | ||
/// current insertion point is threaded through the cleanup, as are | ||
/// 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. | ||
Comment on lines
+139
to
+143
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to do an aligned operator new below now that we can use C++17. But isn't it easier to create a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Honestly, this part of the code really concerns me. We've gone to a lot of effort to have an ultra-efficient allocator because cleanups are being pushed and popped from the stack so frequently, but then every time we pop a cleanup we're copying the memory here. It seems counter-productive. I'm just reluctant to take on a major change to this code because it's been working in classic codegen for a long time. The goal here is to grab a small bit of space on the heap. Until this change six years ago, it was using llvm::AlignedCharArray. We can't use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
+1, if such changes have to happen it might be worth do a separate PR that handles changes in both OG and CIR. |
||
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. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be |
||
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 { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a
errorNYI("SEH")
?Note that I just asked the same thing here: #152802 (comment)