Skip to content

Commit

Permalink
[CIR][CIRGen] Add more tracking skeleton for missing cleanups
Browse files Browse the repository at this point in the history
There are scenarios where we are not emitting cleanups, this commit starts to
pave the way to be more complete in that area. Small addition of skeleton here
plus some fixes.

Both `clang/test/CIR/CodeGen/vla.c` and `clang/test/CIR/CodeGen/nrvo.cpp`
now pass in face of this code path.
  • Loading branch information
bcardosolopes committed Nov 27, 2024
1 parent 70a2723 commit ffa0c9e
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 28 deletions.
44 changes: 42 additions & 2 deletions clang/lib/CIR/CodeGen/CIRGenCleanup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,36 @@ cir::BrOp CIRGenFunction::emitBranchThroughCleanup(mlir::Location Loc,

// Insert a branch: to the cleanup block (unsolved) or to the already
// materialized label. Keep track of unsolved goto's.
return builder.create<BrOp>(Loc, Dest.isValid() ? Dest.getBlock()
: ReturnBlock().getBlock());
auto brOp = builder.create<BrOp>(
Loc, Dest.isValid() ? Dest.getBlock() : ReturnBlock().getBlock());

// Calculate the innermost active normal cleanup.
EHScopeStack::stable_iterator TopCleanup =
EHStack.getInnermostActiveNormalCleanup();

// If we're not in an active normal cleanup scope, or if the
// destination scope is within the innermost active normal cleanup
// scope, we don't need to worry about fixups.
if (TopCleanup == EHStack.stable_end() ||
TopCleanup.encloses(Dest.getScopeDepth())) { // works for invalid
// FIXME(cir): should we clear insertion point here?
return brOp;
}

// If we can't resolve the destination cleanup scope, just add this
// to the current cleanup scope as a branch fixup.
if (!Dest.getScopeDepth().isValid()) {
BranchFixup &Fixup = EHStack.addBranchFixup();
Fixup.destination = Dest.getBlock();
Fixup.destinationIndex = Dest.getDestIndex();
Fixup.initialBranch = brOp;
Fixup.optimisticBranchBlock = nullptr;
// FIXME(cir): should we clear insertion point here?
return brOp;
}

// FIXME(cir): otherwise, thread through all the normal cleanups in scope.
return brOp;
}

/// Emits all the code to cause the given temporary to be cleaned up.
Expand Down Expand Up @@ -574,6 +602,18 @@ void CIRGenFunction::PopCleanupBlocks(

void EHScopeStack::Cleanup::anchor() {}

EHScopeStack::stable_iterator
EHScopeStack::getInnermostActiveNormalCleanup() const {
for (stable_iterator si = getInnermostNormalCleanup(), se = stable_end();
si != se;) {
EHCleanupScope &cleanup = cast<EHCleanupScope>(*find(si));
if (cleanup.isActive())
return si;
si = cleanup.getEnclosingNormalCleanup();
}
return stable_end();
}

/// Push an entry of the given size onto this protected-scope stack.
char *EHScopeStack::allocate(size_t Size) {
Size = llvm::alignTo(Size, ScopeStackAlignment);
Expand Down
10 changes: 4 additions & 6 deletions clang/lib/CIR/CodeGen/CIRGenFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -537,8 +537,8 @@ void CIRGenFunction::finishFunction(SourceLocation EndLoc) {
// the ret after it's been at EndLoc.
if (auto *DI = getDebugInfo())
assert(!cir::MissingFeatures::generateDebugInfo() && "NYI");
// FIXME(cir): vla.c test currently crashes here.
// PopCleanupBlocks(PrologueCleanupDepth);
builder.clearInsertionPoint();
PopCleanupBlocks(PrologueCleanupDepth);
}

// Emit function epilog (to return).
Expand All @@ -561,8 +561,7 @@ void CIRGenFunction::finishFunction(SourceLocation EndLoc) {
assert(!cir::MissingFeatures::emitFunctionEpilog() && "NYI");
assert(!cir::MissingFeatures::emitEndEHSpec() && "NYI");

// FIXME(cir): vla.c test currently crashes here.
// assert(EHStack.empty() && "did not remove all scopes from cleanup stack!");
assert(EHStack.empty() && "did not remove all scopes from cleanup stack!");

// If someone did an indirect goto, emit the indirect goto block at the end of
// the function.
Expand Down Expand Up @@ -1203,8 +1202,7 @@ void CIRGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
}

assert(!cir::MissingFeatures::emitStartEHSpec() && "NYI");
// FIXME(cir): vla.c test currently crashes here.
// PrologueCleanupDepth = EHStack.stable_begin();
PrologueCleanupDepth = EHStack.stable_begin();

// Emit OpenMP specific initialization of the device functions.
if (getLangOpts().OpenMP && CurCodeDecl)
Expand Down
21 changes: 17 additions & 4 deletions clang/lib/CIR/CodeGen/CIRGenFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,24 @@ class CIRGenFunction : public CIRGenTypeCache {
/// require a jump out through normal cleanups.
struct JumpDest {
JumpDest() = default;
JumpDest(mlir::Block *Block) : Block(Block) {}
JumpDest(mlir::Block *block, EHScopeStack::stable_iterator depth = {},
unsigned index = 0)
: block(block) {}

bool isValid() const { return block != nullptr; }
mlir::Block *getBlock() const { return block; }
EHScopeStack::stable_iterator getScopeDepth() const { return scopeDepth; }
unsigned getDestIndex() const { return index; }

// This should be used cautiously.
void setScopeDepth(EHScopeStack::stable_iterator depth) {
scopeDepth = depth;
}

bool isValid() const { return Block != nullptr; }
mlir::Block *getBlock() const { return Block; }
mlir::Block *Block = nullptr;
private:
mlir::Block *block = nullptr;
EHScopeStack::stable_iterator scopeDepth;
unsigned index;
};

/// Track mlir Blocks for each C/C++ label.
Expand Down
32 changes: 16 additions & 16 deletions clang/lib/CIR/CodeGen/EHScopeStack.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,22 @@ class CIRGenFunction;
/// the innermost cleanup. When a (normal) cleanup is popped, any
/// unresolved fixups in that scope are threaded through the cleanup.
struct BranchFixup {
// /// The block containing the terminator which needs to be modified
// /// into a switch if this fixup is resolved into the current scope.
// /// If null, LatestBranch points directly to the destination.
// llvm::BasicBlock *OptimisticBranchBlock;

// /// The ultimate destination of the branch.
// ///
// /// This can be set to null to indicate that this fixup was
// /// successfully resolved.
// llvm::BasicBlock *Destination;

// /// The destination index value.
// unsigned DestinationIndex;

// /// The initial branch of the fixup.
// llvm::BranchInst *InitialBranch;
/// The block containing the terminator which needs to be modified
/// into a switch if this fixup is resolved into the current scope.
/// If null, LatestBranch points directly to the destination.
mlir::Block *optimisticBranchBlock = nullptr;

/// The ultimate destination of the branch.
///
/// This can be set to null to indicate that this fixup was
/// successfully resolved.
mlir::Block *destination = nullptr;

/// The destination index value.
unsigned destinationIndex = 0;

/// The initial branch of the fixup.
cir::BrOp initialBranch = {};
};

template <class T> struct InvariantValue {
Expand Down

0 comments on commit ffa0c9e

Please sign in to comment.