-
Notifications
You must be signed in to change notification settings - Fork 14.8k
[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 all commits
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,101 @@ void EHScopeStack::deallocate(size_t size) { | |
} | ||
|
||
void *EHScopeStack::pushCleanup(CleanupKind kind, size_t size) { | ||
char *buffer = allocate(size); | ||
char *buffer = allocate(EHCleanupScope::getSizeForCleanupSize(size)); | ||
bool isEHCleanup = kind & EHCleanup; | ||
bool isLifetimeMarker = kind & LifetimeMarker; | ||
|
||
// 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()); | ||
assert(!cir::MissingFeatures::innermostEHScope()); | ||
|
||
return buffer; | ||
EHCleanupScope *scope = new (buffer) EHCleanupScope(size); | ||
|
||
if (isLifetimeMarker) | ||
cgf->cgm.errorNYI("push lifetime marker cleanup"); | ||
|
||
// With Windows -EHa, Invoke llvm.seh.scope.begin() for EHCleanup | ||
if (cgf->getLangOpts().EHAsynch && isEHCleanup && !isLifetimeMarker && | ||
cgf->getTarget().getCXXABI().isMicrosoft()) | ||
cgf->cgm.errorNYI("push seh cleanup"); | ||
|
||
return scope->getCleanupBuffer(); | ||
} | ||
|
||
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 mlir::Block *getCurCleanupBlock(CIRGenFunction &cgf) { | ||
mlir::OpBuilder::InsertionGuard guard(cgf.getBuilder()); | ||
mlir::Block *cleanup = | ||
cgf.curLexScope->getOrCreateCleanupBlock(cgf.getBuilder()); | ||
return cleanup; | ||
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()); | ||
|
||
// Remember activation information. | ||
bool isActive = scope.isActive(); | ||
|
||
assert(!cir::MissingFeatures::ehCleanupBranchFixups()); | ||
|
||
// 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(); | ||
// - whether there's a fallthrough | ||
mlir::Block *fallthroughSource = builder.getInsertionBlock(); | ||
bool hasFallthrough = fallthroughSource != nullptr && isActive; | ||
|
||
// The eventual implementation here will use the EHCleanupScope helper class. | ||
assert(!cir::MissingFeatures::ehCleanupScope()); | ||
bool requiresNormalCleanup = scope.isNormalCleanup() && hasFallthrough; | ||
|
||
mlir::OpBuilder::InsertionGuard guard(builder); | ||
// 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. | ||
|
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)