[CIR][CIRGen] Properly link multiple level of cleanups

Generalize approach and be able to tie together cleanups with their matching throwing calls. Before this the dtors were not really emitted in the proper order. LLVM support for this still hits a NYI, so nothing special here on the LLVM lowering side.

Author: bcardosolopes

clang/include/clang/CIR/Dialect/IR/CIROps.td

@@ -3515,27 +3515,26 @@ def TryOp : CIR_Op<"try",
  `synthetic`: use `cir.try` to represent try/catches not originally
  present in the source code (e.g. `g = new Class` under `-fexceptions`).
 
+ `cleanup`: signal to targets (LLVM for now) that this try/catch, needs
+ to specially tag their landing pads as needing "cleanup".
+
  Example: TBD
  ```
  }];
 
- let arguments = (ins UnitAttr:$synthetic,
+ let arguments = (ins UnitAttr:$synthetic, UnitAttr:$cleanup,
  OptionalAttr<ArrayAttr>:$catch_types);
  let regions = (region AnyRegion:$try_region,
- AnyRegion:$cleanup_region,
  VariadicRegion<AnyRegion>:$catch_regions);
 
  let assemblyFormat = [{
- (`synthetic` $synthetic^)? $try_region
- `cleanup` $cleanup_region
+ (`synthetic` $synthetic^)?
+ (`cleanup` $cleanup^)?
+ $try_region
  custom<CatchRegions>($catch_regions, $catch_types)
  attr-dict
  }];
 
- let extraClassDeclaration = [{
- bool isCleanupActive() { return !getCleanupRegion().empty(); }
- }];
-
  // Everything already covered elsewhere.
  let hasVerifier = 0;
  let builders = [

clang/lib/CIR/CodeGen/CIRGenCleanup.cpp

@@ -158,9 +158,9 @@ void CIRGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
 
  // Check whether we need an EH cleanup. This is only true if we've
  // generated a lazy EH cleanup block.
- auto *EHEntry = Scope.getCachedEHDispatchBlock();
- assert(Scope.hasEHBranches() == (EHEntry != nullptr));
- bool RequiresEHCleanup = (EHEntry != nullptr);
+ auto *ehEntry = Scope.getCachedEHDispatchBlock();
+ assert(Scope.hasEHBranches() == (ehEntry != nullptr));
+ bool RequiresEHCleanup = (ehEntry != nullptr);
  EHScopeStack::stable_iterator EHParent = Scope.getEnclosingEHScope();
 
  // Check the three conditions which might require a normal cleanup:
@@ -300,8 +300,8 @@ void CIRGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
  }
 
  assert(tryOp && "expected available cir.try");
- auto *NextAction = getEHDispatchBlock(EHParent, tryOp);
- (void)NextAction;
+ auto *nextAction = getEHDispatchBlock(EHParent, tryOp);
+ (void)nextAction;
 
  // Push a terminate scope or cleanupendpad scope around the potentially
  // throwing cleanups. For funclet EH personalities, the cleanupendpad models
@@ -328,23 +328,34 @@ void CIRGenFunction::PopCleanupBlock(bool FallthroughIsBranchThrough) {
  if (EHActiveFlag.isValid() || IsActive) {
  cleanupFlags.setIsForEHCleanup();
  mlir::OpBuilder::InsertionGuard guard(builder);
- if (!tryOp.isCleanupActive())
- builder.createBlock(&tryOp.getCleanupRegion());
- mlir::Block *cleanup = &tryOp.getCleanupRegion().back();
- if (cleanup->empty()) {
- builder.setInsertionPointToEnd(cleanup);
- builder.createYield(tryOp.getLoc());
- }
 
- auto yield = cast<YieldOp>(cleanup->getTerminator());
+ auto yield = cast<YieldOp>(ehEntry->getTerminator());
  builder.setInsertionPoint(yield);
  buildCleanup(*this, Fn, cleanupFlags, EHActiveFlag);
  }
 
- // In LLVM traditional codegen, here's where it branches off to
- // NextAction.
  if (CPI)
  llvm_unreachable("NYI");
+ else {
+ // In LLVM traditional codegen, here's where it branches off to
+ // nextAction. CIR does not have a flat layout at this point, so
+ // instead patch all the landing pads that need to run this cleanup
+ // as well.
+ mlir::Block *currBlock = ehEntry;
+ while (currBlock && cleanupsToPatch.contains(currBlock)) {
+ mlir::OpBuilder::InsertionGuard guard(builder);
+ mlir::Block *blockToPatch = cleanupsToPatch[currBlock];
+ auto currYield = cast<YieldOp>(blockToPatch->getTerminator());
+ builder.setInsertionPoint(currYield);
+ buildCleanup(*this, Fn, cleanupFlags, EHActiveFlag);
+ currBlock = blockToPatch;
+ }
+
+ // The nextAction is yet to be populated, register that this
+ // cleanup should also incorporate any cleanup from nextAction
+ // when available.
+ cleanupsToPatch[nextAction] = ehEntry;
+ }
 
  // Leave the terminate scope.
  if (PushedTerminate)

clang/lib/CIR/CodeGen/CIRGenException.cpp

@@ -417,7 +417,7 @@ static void buildCatchDispatchBlock(CIRGenFunction &CGF,
  // that catch-all as the dispatch block.
  if (catchScope.getNumHandlers() == 1 &&
  catchScope.getHandler(0).isCatchAll()) {
- assert(dispatchBlock == catchScope.getHandler(0).Block);
+ // assert(dispatchBlock == catchScope.getHandler(0).Block);
  return;
  }
 
@@ -721,8 +721,7 @@ mlir::Operation *CIRGenFunction::buildLandingPad(mlir::cir::TryOp tryOp) {
 
  // Otherwise, signal that we at least have cleanups.
  } else if (hasCleanup) {
- if (!tryOp.isCleanupActive())
- builder.createBlock(&tryOp.getCleanupRegion());
+ tryOp.setCleanup(true);
  }
 
  assert((clauses.size() > 0 || hasCleanup) && "no catch clauses!");
@@ -739,9 +738,7 @@ mlir::Operation *CIRGenFunction::buildLandingPad(mlir::cir::TryOp tryOp) {
  mlir::ArrayAttr::get(builder.getContext(), clauses));
 
  // In traditional LLVM codegen. this tells the backend how to generate the
- // landing pad by generating a branch to the dispatch block. In CIR the same
- // function is called to gather some state, but this block info it's not
- // useful per-se.
+ // landing pad by generating a branch to the dispatch block.
  mlir::Block *dispatch =
  getEHDispatchBlock(EHStack.getInnermostEHScope(), tryOp);
  (void)dispatch;
@@ -772,28 +769,25 @@ CIRGenFunction::getEHDispatchBlock(EHScopeStack::stable_iterator si,
  if (!dispatchBlock) {
  switch (scope.getKind()) {
  case EHScope::Catch: {
- // Apply a special case to a single catch-all.
- EHCatchScope &catchScope = cast<EHCatchScope>(scope);
- if (catchScope.getNumHandlers() == 1 &&
- catchScope.getHandler(0).isCatchAll()) {
- dispatchBlock = catchScope.getHandler(0).Block;
-
- // Otherwise, make a dispatch block.
- } else {
- // As said in the function comment, just signal back we
- // have something - even though the block value doesn't
- // have any real meaning.
- dispatchBlock = catchScope.getHandler(0).Block;
- assert(dispatchBlock && "find another approach to signal");
+ // LLVM does some optimization with branches here, CIR just keep track of
+ // the corresponding calls.
+ assert(callWithExceptionCtx && "expected call information");
+ {
+ mlir::OpBuilder::InsertionGuard guard(getBuilder());
+ assert(callWithExceptionCtx.getCleanup().empty() &&
+ "one per call: expected empty region at this point");
+ dispatchBlock = builder.createBlock(&callWithExceptionCtx.getCleanup());
+ builder.createYield(callWithExceptionCtx.getLoc());
  }
  break;
  }
 
  case EHScope::Cleanup: {
- assert(tryOp && "expected cir.try available");
  assert(callWithExceptionCtx && "expected call information");
  {
  mlir::OpBuilder::InsertionGuard guard(getBuilder());
+ assert(callWithExceptionCtx.getCleanup().empty() &&
+ "one per call: expected empty region at this point");
  dispatchBlock = builder.createBlock(&callWithExceptionCtx.getCleanup());
  builder.createYield(callWithExceptionCtx.getLoc());
  }

clang/lib/CIR/CodeGen/CIRGenFunction.h

@@ -1755,6 +1755,7 @@ class CIRGenFunction : public CIRGenTypeCache {
  mlir::cir::TryOp tryOp);
  /// Unified block containing a call to cir.resume
  mlir::Block *ehResumeBlock = nullptr;
+ llvm::DenseMap<mlir::Block *, mlir::Block *> cleanupsToPatch;
 
  /// The cleanup depth enclosing all the cleanups associated with the
  /// parameters.

clang/lib/CIR/Dialect/IR/CIRDialect.cpp

@@ -1237,9 +1237,6 @@ void TryOp::build(
  // Try body region
  Region *tryBodyRegion = result.addRegion();
 
- // Try cleanup region
- result.addRegion();
-
  // Create try body region and set insertion point
  builder.createBlock(tryBodyRegion);
  tryBodyBuilder(builder, result.location);
@@ -1257,7 +1254,7 @@ void TryOp::getSuccessorRegions(mlir::RegionBranchPoint point,
 
  // If the condition isn't constant, both regions may be executed.
  regions.push_back(RegionSuccessor(&getTryRegion()));
- regions.push_back(RegionSuccessor(&getCleanupRegion()));
+
  // FIXME: optimize, ideas include:
  // - If we know a target function never throws a specific type, we can
  // remove the catch handler.
@@ -3004,7 +3001,7 @@ void printCallCommon(Operation *op, mlir::Value indirectCallee,
  auto call = dyn_cast<mlir::cir::CallOp>(op);
  assert(call && "expected regular call");
  if (!call.getCleanup().empty()) {
- state << "cleanup ";
+ state << " cleanup ";
  state.printRegion(call.getCleanup());
  }
  }

clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp

@@ -294,10 +294,10 @@ class CIRTryOpFlattening : public mlir::OpRewritePattern<mlir::cir::TryOp> {
  return mlir::ArrayAttr::get(caseAttrList.getContext(), symbolList);
  }
 
- mlir::Block *buildCatchers(mlir::cir::TryOp tryOp,
-   mlir::PatternRewriter &rewriter,
-  mlir::Block *afterBody,
-  mlir::Block *afterTry) const {
+ mlir::Block *
+ buildCatchers(mlir::cir::TryOp tryOp, mlir::PatternRewriter &rewriter,
+ mlir::Block *afterBody, mlir::Block *afterTry,
+ SmallVectorImpl<mlir::cir::CallOp> &callsToRewrite) const {
  auto loc = tryOp.getLoc();
  // Replace the tryOp return with a branch that jumps out of the body.
  rewriter.setInsertionPointToEnd(afterBody);
@@ -317,22 +317,22 @@ class CIRTryOpFlattening : public mlir::OpRewritePattern<mlir::cir::TryOp> {
  mlir::ArrayAttr symlist = collectTypeSymbols(tryOp);
  auto inflightEh = rewriter.create<mlir::cir::EhInflightOp>(
  loc, exceptionPtrType, typeIdType,
- tryOp.isCleanupActive() ? mlir::UnitAttr::get(tryOp.getContext())
- : nullptr,
+ tryOp.getCleanup() ? mlir::UnitAttr::get(tryOp.getContext()) : nullptr,
  symlist);
  auto selector = inflightEh.getTypeId();
  auto exceptionPtr = inflightEh.getExceptionPtr();
 
  // Time to emit cleanup's.
- if (tryOp.isCleanupActive()) {
- assert(tryOp.getCleanupRegion().getBlocks().size() == 1 &&
+ if (tryOp.getCleanup()) {
+ assert(callsToRewrite.size() == 1 &&
  "NYI: if this isn't enough, move region instead");
  // TODO(cir): this might need to be duplicated instead of consumed since
  // for user-written try/catch we want these cleanups to also run when the
  // regular try scope adjurns (in case no exception is triggered).
  assert(tryOp.getSynthetic() &&
  "not implemented for user written try/catch");
- mlir::Block *cleanupBlock = &tryOp.getCleanupRegion().getBlocks().back();
+ mlir::Block *cleanupBlock =
+ &callsToRewrite[0].getCleanup().getBlocks().back();
  auto cleanupYield =
  cast<mlir::cir::YieldOp>(cleanupBlock->getTerminator());
  cleanupYield->erase();
@@ -465,7 +465,7 @@ class CIRTryOpFlattening : public mlir::OpRewritePattern<mlir::cir::TryOp> {
 
  // Build catchers.
  mlir::Block *landingPad =
- buildCatchers(tryOp, rewriter, afterBody, afterTry);
+ buildCatchers(tryOp, rewriter, afterBody, afterTry, callsToRewrite);
  rewriter.eraseOp(tryOp);
 
  // Rewrite calls.

clang/test/CIR/CodeGen/global-new.cpp

@@ -27,11 +27,11 @@ e *g = new e(0);
 // CIR_AFTER: {{%.*}} = cir.const #cir.int<1> : !u64i
 // CIR_AFTER: {{%.*}} = cir.call @_Znwm(%1) : (!u64i) -> !cir.ptr<!void>
 
-// CIR_EH: cir.try synthetic {
-// CIR_EH: cir.call exception @_ZN1eC1Ei
-// CIR_EH: cir.yield
-// CIR_EH: } cleanup {
-// CIR_EH: cir.call @_ZdlPvm
+// CIR_EH: cir.try synthetic cleanup {
+// CIR_EH: cir.call exception @_ZN1eC1Ei{{.*}} cleanup {
+// CIR_EH:  cir.call @_ZdlPvm
+// CIR_EH:  cir.yield
+// CIR_EH: }
 // CIR_EH: cir.yield
 // CIR_EH: } catch [#cir.unwind {
 // CIR_EH: cir.resume

clang/test/CIR/CodeGen/paren-list-init.cpp

@@ -43,15 +43,15 @@ void make1() {
 // CIR_EH: cir.scope {
 // CIR_EH: %1 = cir.alloca ![[S1]], !cir.ptr<![[S1]]>, ["agg.tmp.ensured"]
 // CIR_EH: %2 = cir.get_member %1[0] {name = "v"} : !cir.ptr<![[S1]]> -> !cir.ptr<![[VecType]]>
-// CIR_EH: cir.try synthetic {
+// CIR_EH: cir.try synthetic cleanup {
 
 // Call v move ctor
-// CIR_EH: cir.call exception @_ZN3VecC1EOS_(%2, %[[VEC]]) : (!cir.ptr<![[VecType]]>, !cir.ptr<![[VecType]]>) -> ()
-// CIR_EH: cir.yield
-// CIR_EH: } cleanup {
+// CIR_EH: cir.call exception @_ZN3VecC1EOS_{{.*}} cleanup {
 
 // Destroy v after v move ctor throws
-// CIR_EH: cir.call @_ZN3VecD1Ev(%[[VEC]]) : (!cir.ptr<![[VecType]]>) -> ()
+// CIR_EH: cir.call @_ZN3VecD1Ev(%[[VEC]])
+// CIR_EH: cir.yield
+// CIR_EH: }
 // CIR_EH: cir.yield
 // CIR_EH: } catch [#cir.unwind {
 // CIR_EH: cir.resume

clang/test/CIR/CodeGen/try-catch-dtors.cpp

@@ -27,7 +27,6 @@ void yo() {
 // CIR: cir.call exception @_ZN3VecC1Ev(%[[VADDR]]) : (!cir.ptr<![[VecTy]]>) -> ()
 // CIR: cir.call @_ZN3VecD1Ev(%[[VADDR]]) : (!cir.ptr<![[VecTy]]>) -> ()
 // CIR: cir.yield
-// CIR: } cleanup {
 // CIR: } catch [type #cir.all {
 // CIR: cir.catch_param -> !cir.ptr<!void>
 // CIR: }]
@@ -75,6 +74,16 @@ void yo2() {
  r++;
  }
 }
+
+void yo3(bool x) {
+ int r = 1;
+ try {
+ Vec v1, v2, v3, v4;
+ } catch (...) {
+ r++;
+ }
+}
+
 #endif
 
 // CIR: cir.func @_Z3yo2v()
@@ -84,18 +93,49 @@ void yo2() {
 // CIR: cir.call exception @_ZN3VecC1Ev
 // CIR: cir.scope {
 // CIR: cir.alloca ![[S1:.*]], !cir.ptr<![[S1:.*]]>, ["agg.tmp.ensured"]
-// CIR: cir.call exception @_ZN3VecC1EOS_
+// CIR: cir.call exception @_ZN3VecC1EOS_{{.*}} cleanup {
+// CIR: cir.call @_ZN3VecD1Ev
+// CIR: cir.yield
 // CIR: cir.call @_ZN2S1D2Ev
 // CIR: }
 // CIR: cir.call @_ZN3VecD1Ev
 // CIR: cir.yield
-// CIR: } cleanup {
-// CIR: cir.call @_ZN3VecD1Ev
-// CIR: cir.yield
 // CIR: } catch [type #cir.all {
 // CIR: cir.catch_param -> !cir.ptr<!void>
 // CIR: cir.yield
 // CIR: }]
 // CIR: }
 // CIR: cir.return
-// CIR: }
+// CIR: }
+
+// CIR: cir.scope {
+// CIR: %[[V1:.*]] = cir.alloca ![[VecTy]], !cir.ptr<![[VecTy]]>, ["v1"
+// CIR: %[[V2:.*]] = cir.alloca ![[VecTy]], !cir.ptr<![[VecTy]]>, ["v2"
+// CIR: %[[V3:.*]] = cir.alloca ![[VecTy]], !cir.ptr<![[VecTy]]>, ["v3"
+// CIR: %[[V4:.*]] = cir.alloca ![[VecTy]], !cir.ptr<![[VecTy]]>, ["v4"
+// CIR: cir.try {
+// CIR: cir.call exception @_ZN3VecC1Ev(%[[V1]]) : (!cir.ptr<![[VecTy]]>) -> ()
+// CIR: cir.call exception @_ZN3VecC1Ev(%[[V2]]) : (!cir.ptr<![[VecTy]]>) -> () cleanup {
+// CIR: cir.call @_ZN3VecD1Ev(%[[V1]]) : (!cir.ptr<![[VecTy]]>) -> ()
+// CIR: cir.yield
+// CIR: }
+// CIR: cir.call exception @_ZN3VecC1Ev(%[[V3]]) : (!cir.ptr<![[VecTy]]>) -> () cleanup {
+// CIR: cir.call @_ZN3VecD1Ev(%[[V2]]) : (!cir.ptr<![[VecTy]]>) -> ()
+// CIR: cir.call @_ZN3VecD1Ev(%[[V1]]) : (!cir.ptr<![[VecTy]]>) -> ()
+// CIR: cir.yield
+// CIR: }
+// CIR: cir.call exception @_ZN3VecC1Ev(%[[V4]]) : (!cir.ptr<![[VecTy]]>) -> () cleanup {
+// CIR: cir.call @_ZN3VecD1Ev(%[[V3]]) : (!cir.ptr<![[VecTy]]>) -> ()
+// CIR: cir.call @_ZN3VecD1Ev(%[[V2]]) : (!cir.ptr<![[VecTy]]>) -> ()
+// CIR: cir.call @_ZN3VecD1Ev(%[[V1]]) : (!cir.ptr<![[VecTy]]>) -> ()
+// CIR: cir.yield
+// CIR: }
+// CIR: cir.call @_ZN3VecD1Ev(%[[V4]]) : (!cir.ptr<![[VecTy]]>) -> ()
+// CIR: cir.call @_ZN3VecD1Ev(%[[V3]]) : (!cir.ptr<![[VecTy]]>) -> ()
+// CIR: cir.call @_ZN3VecD1Ev(%[[V2]]) : (!cir.ptr<![[VecTy]]>) -> ()
+// CIR: cir.call @_ZN3VecD1Ev(%[[V1]]) : (!cir.ptr<![[VecTy]]>) -> ()
+// CIR: cir.yield
+// CIR: } catch [type #cir.all {
+// CIR: }]
+// CIR: }
+// CIR: cir.return

clang/test/CIR/CodeGen/try-catch.cpp

@@ -21,7 +21,6 @@ unsigned long long tc() {
  a++;
 
  } catch (int idx) {
- // CHECK: } cleanup {
  // CHECK: } catch [type #cir.global_view<@_ZTIi> : !cir.ptr<!u8i> {
  // CHECK: %[[catch_idx_addr:.*]] = cir.catch_param -> !cir.ptr<!s32i>
  // CHECK: %[[idx_load:.*]] = cir.load %[[catch_idx_addr]] : !cir.ptr<!s32i>, !s32i