Skip to content

Commit ffa0c9e

Browse files
committed
[CIR][CIRGen] Add more tracking skeleton for missing cleanups
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.
1 parent 70a2723 commit ffa0c9e

File tree

4 files changed

+79
-28
lines changed

4 files changed

+79
-28
lines changed

clang/lib/CIR/CodeGen/CIRGenCleanup.cpp

+42-2
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,36 @@ cir::BrOp CIRGenFunction::emitBranchThroughCleanup(mlir::Location Loc,
4242

4343
// Insert a branch: to the cleanup block (unsolved) or to the already
4444
// materialized label. Keep track of unsolved goto's.
45-
return builder.create<BrOp>(Loc, Dest.isValid() ? Dest.getBlock()
46-
: ReturnBlock().getBlock());
45+
auto brOp = builder.create<BrOp>(
46+
Loc, Dest.isValid() ? Dest.getBlock() : ReturnBlock().getBlock());
47+
48+
// Calculate the innermost active normal cleanup.
49+
EHScopeStack::stable_iterator TopCleanup =
50+
EHStack.getInnermostActiveNormalCleanup();
51+
52+
// If we're not in an active normal cleanup scope, or if the
53+
// destination scope is within the innermost active normal cleanup
54+
// scope, we don't need to worry about fixups.
55+
if (TopCleanup == EHStack.stable_end() ||
56+
TopCleanup.encloses(Dest.getScopeDepth())) { // works for invalid
57+
// FIXME(cir): should we clear insertion point here?
58+
return brOp;
59+
}
60+
61+
// If we can't resolve the destination cleanup scope, just add this
62+
// to the current cleanup scope as a branch fixup.
63+
if (!Dest.getScopeDepth().isValid()) {
64+
BranchFixup &Fixup = EHStack.addBranchFixup();
65+
Fixup.destination = Dest.getBlock();
66+
Fixup.destinationIndex = Dest.getDestIndex();
67+
Fixup.initialBranch = brOp;
68+
Fixup.optimisticBranchBlock = nullptr;
69+
// FIXME(cir): should we clear insertion point here?
70+
return brOp;
71+
}
72+
73+
// FIXME(cir): otherwise, thread through all the normal cleanups in scope.
74+
return brOp;
4775
}
4876

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

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

605+
EHScopeStack::stable_iterator
606+
EHScopeStack::getInnermostActiveNormalCleanup() const {
607+
for (stable_iterator si = getInnermostNormalCleanup(), se = stable_end();
608+
si != se;) {
609+
EHCleanupScope &cleanup = cast<EHCleanupScope>(*find(si));
610+
if (cleanup.isActive())
611+
return si;
612+
si = cleanup.getEnclosingNormalCleanup();
613+
}
614+
return stable_end();
615+
}
616+
577617
/// Push an entry of the given size onto this protected-scope stack.
578618
char *EHScopeStack::allocate(size_t Size) {
579619
Size = llvm::alignTo(Size, ScopeStackAlignment);

clang/lib/CIR/CodeGen/CIRGenFunction.cpp

+4-6
Original file line numberDiff line numberDiff line change
@@ -537,8 +537,8 @@ void CIRGenFunction::finishFunction(SourceLocation EndLoc) {
537537
// the ret after it's been at EndLoc.
538538
if (auto *DI = getDebugInfo())
539539
assert(!cir::MissingFeatures::generateDebugInfo() && "NYI");
540-
// FIXME(cir): vla.c test currently crashes here.
541-
// PopCleanupBlocks(PrologueCleanupDepth);
540+
builder.clearInsertionPoint();
541+
PopCleanupBlocks(PrologueCleanupDepth);
542542
}
543543

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

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

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

12051204
assert(!cir::MissingFeatures::emitStartEHSpec() && "NYI");
1206-
// FIXME(cir): vla.c test currently crashes here.
1207-
// PrologueCleanupDepth = EHStack.stable_begin();
1205+
PrologueCleanupDepth = EHStack.stable_begin();
12081206

12091207
// Emit OpenMP specific initialization of the device functions.
12101208
if (getLangOpts().OpenMP && CurCodeDecl)

clang/lib/CIR/CodeGen/CIRGenFunction.h

+17-4
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,24 @@ class CIRGenFunction : public CIRGenTypeCache {
7676
/// require a jump out through normal cleanups.
7777
struct JumpDest {
7878
JumpDest() = default;
79-
JumpDest(mlir::Block *Block) : Block(Block) {}
79+
JumpDest(mlir::Block *block, EHScopeStack::stable_iterator depth = {},
80+
unsigned index = 0)
81+
: block(block) {}
82+
83+
bool isValid() const { return block != nullptr; }
84+
mlir::Block *getBlock() const { return block; }
85+
EHScopeStack::stable_iterator getScopeDepth() const { return scopeDepth; }
86+
unsigned getDestIndex() const { return index; }
87+
88+
// This should be used cautiously.
89+
void setScopeDepth(EHScopeStack::stable_iterator depth) {
90+
scopeDepth = depth;
91+
}
8092

81-
bool isValid() const { return Block != nullptr; }
82-
mlir::Block *getBlock() const { return Block; }
83-
mlir::Block *Block = nullptr;
93+
private:
94+
mlir::Block *block = nullptr;
95+
EHScopeStack::stable_iterator scopeDepth;
96+
unsigned index;
8497
};
8598

8699
/// Track mlir Blocks for each C/C++ label.

clang/lib/CIR/CodeGen/EHScopeStack.h

+16-16
Original file line numberDiff line numberDiff line change
@@ -34,22 +34,22 @@ class CIRGenFunction;
3434
/// the innermost cleanup. When a (normal) cleanup is popped, any
3535
/// unresolved fixups in that scope are threaded through the cleanup.
3636
struct BranchFixup {
37-
// /// The block containing the terminator which needs to be modified
38-
// /// into a switch if this fixup is resolved into the current scope.
39-
// /// If null, LatestBranch points directly to the destination.
40-
// llvm::BasicBlock *OptimisticBranchBlock;
41-
42-
// /// The ultimate destination of the branch.
43-
// ///
44-
// /// This can be set to null to indicate that this fixup was
45-
// /// successfully resolved.
46-
// llvm::BasicBlock *Destination;
47-
48-
// /// The destination index value.
49-
// unsigned DestinationIndex;
50-
51-
// /// The initial branch of the fixup.
52-
// llvm::BranchInst *InitialBranch;
37+
/// The block containing the terminator which needs to be modified
38+
/// into a switch if this fixup is resolved into the current scope.
39+
/// If null, LatestBranch points directly to the destination.
40+
mlir::Block *optimisticBranchBlock = nullptr;
41+
42+
/// The ultimate destination of the branch.
43+
///
44+
/// This can be set to null to indicate that this fixup was
45+
/// successfully resolved.
46+
mlir::Block *destination = nullptr;
47+
48+
/// The destination index value.
49+
unsigned destinationIndex = 0;
50+
51+
/// The initial branch of the fixup.
52+
cir::BrOp initialBranch = {};
5353
};
5454

5555
template <class T> struct InvariantValue {

0 commit comments

Comments
 (0)