[CIR][CIRGen][NFC] Exceptions: redesign cir.catch in terms of cir.try

Simplify things a bit and remove the propagation of exception in terms of a
returned exception value. This also merges cir.catch as part of cir.try and
remove the former.

Everything that didn't work still doesn't work, nothing new added here. The
CatchOp parsing was wrong even before this commit, so for now add an XFAIL with
the complete testcase we want to be able to parse soon (incremental work will
fix this) in clang/test/CIR/IR/exceptions.cir.

More cleanups to come as well, this is just the first part.

Author: bcardosolopes

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

@@ -780,8 +780,8 @@ def ConditionOp : CIR_Op<"condition", [
 
 def YieldOp : CIR_Op<"yield", [ReturnLike, Terminator,
     ParentOneOf<["IfOp", "ScopeOp", "SwitchOp", "WhileOp", "ForOp", "AwaitOp",
-                 "TernaryOp", "GlobalOp", "DoWhileOp", "CatchOp", "TryOp",
-                 "ArrayCtor", "ArrayDtor"]>]> {
+                 "TernaryOp", "GlobalOp", "DoWhileOp", "TryOp", "ArrayCtor",
+                 "ArrayDtor"]>]> {
   let summary = "Represents the default branching behaviour of a region";
   let description = [{
     The `cir.yield` operation terminates regions on different CIR operations,
@@ -871,7 +871,7 @@ def ContinueOp : CIR_Op<"continue", [Terminator]> {
 //===----------------------------------------------------------------------===//
 
 def ResumeOp : CIR_Op<"resume", [ReturnLike, Terminator,
-                                 ParentOneOf<["CatchOp"]>]> {
+                                 ParentOneOf<["TryOp"]>]> {
   let summary = "Resumes execution after not catching exceptions";
   let description = [{
     The `cir.resume` operation terminates a region on `cir.catch`, "resuming"
@@ -3205,6 +3205,17 @@ def AwaitOp : CIR_Op<"await",
 // TryOp
 //===----------------------------------------------------------------------===//
 
+// Represents the unwind region where unwind continues or
+// the program std::terminate's.
+def CatchUnwind : CIRUnitAttr<"CatchUnwind", "unwind"> {
+  let storageType = [{ CatchUnwind }];
+}
+
+// Represents the catch_all region.
+def CatchAllAttr : CIRUnitAttr<"CatchAll", "all"> {
+  let storageType = [{ CatchAllAttr }];
+}
+
 def TryOp : CIR_Op<"try",
        [DeclareOpInterfaceMethods<RegionBranchOpInterface>,
         RecursivelySpeculatable, AutomaticAllocationScope,
@@ -3228,67 +3239,26 @@ def TryOp : CIR_Op<"try",
     ```
   }];
 
-  let regions = (region AnyRegion:$body);
-  let results = (outs ExceptionInfoPtr:$result);
+  let arguments = (ins OptionalAttr<ArrayAttr>:$catch_types);
+  let regions = (region AnyRegion:$try_region,
+                        VariadicRegion<AnyRegion>:$catch_regions);
 
   let assemblyFormat = [{
-    $body `:` functional-type(operands, results) attr-dict
+    $try_region
+    custom<CatchRegions>($catch_regions, $catch_types)
+    attr-dict
   }];
 
   // Everything already covered elsewhere.
   let hasVerifier = 0;
   let builders = [
     OpBuilder<(ins
-              "function_ref<void(OpBuilder &, Type &, Location)>":$tryBuilder)>,
+              "function_ref<void(OpBuilder &, Location)>":$tryBuilder,
+              "function_ref<void(OpBuilder &, Location, OperationState &)>"
+              :$catchBuilder)>,
   ];
 }
 
-//===----------------------------------------------------------------------===//
-// CatchOp
-//===----------------------------------------------------------------------===//
-
-// Represents the unwind region where unwind continues or
-// the program std::terminate's.
-def CatchUnwind : CIRUnitAttr<"CatchUnwind", "unwind"> {
-  let storageType = [{ CatchUnwind }];
-}
-
-// Represents the catch_all region.
-def CatchAllAttr : CIRUnitAttr<"CatchAll", "all"> {
-  let storageType = [{ CatchAllAttr }];
-}
-
-def CatchOp : CIR_Op<"catch",
-      [SameVariadicOperandSize,
-       DeclareOpInterfaceMethods<RegionBranchOpInterface>,
-       RecursivelySpeculatable, NoRegionArguments]> {
-  let summary = "Catch operation";
-  let description = [{
-  }];
-
-  let arguments = (ins CIR_AnyType:$exception_info,
-                       OptionalAttr<ArrayAttr>:$catchers);
-  let regions = (region VariadicRegion<AnyRegion>:$regions);
-
-  // Already verified elsewhere
-  let hasVerifier = 0;
-
-  let skipDefaultBuilders = 1;
-  let builders = [
-    OpBuilder<(ins
-      "Value":$exception_info,
-      "function_ref<void(OpBuilder &, Location, OperationState &)>"
-      :$catchBuilder)>
-  ];
-
-  let assemblyFormat = [{
-    `(`
-      $exception_info `:` type($exception_info) `,`
-      custom<CatchOp>($regions, $catchers)
-    `)` attr-dict
-  }];
-}
-
 //===----------------------------------------------------------------------===//
 // CatchParamOp
 //===----------------------------------------------------------------------===//
@@ -3306,10 +3276,9 @@ def CatchParamOp : CIR_Op<"catch_param"> {
     ```
   }];
 
-  let arguments = (ins ExceptionInfoPtr:$exception_info);
   let results = (outs CIR_AnyType:$param);
   let assemblyFormat = [{
-    `(` $exception_info `)` `->` qualified(type($param)) attr-dict
+    `->` qualified(type($param)) attr-dict
   }];
 
   let hasVerifier = 0;

clang/lib/CIR/CodeGen/CIRGenCleanup.h

@@ -206,7 +206,7 @@ class EHCatchScope : public EHScope {
   // 'takeHandler' or some such function which removes ownership from the
   // EHCatchScope object if the handlers should live longer than EHCatchScope.
   void clearHandlerBlocks() {
-    // The blocks are owned by CatchOp, nothing to delete.
+    // The blocks are owned by TryOp, nothing to delete.
   }
 
   typedef const Handler *iterator;

clang/lib/CIR/CodeGen/CIRGenException.cpp

@@ -257,8 +257,9 @@ mlir::Block *CIRGenFunction::getEHResumeBlock(bool isCleanup) {
   // pointer but only use it to denote we're tracking things, but there
   // shouldn't be any changes to that block after work done in this function.
   auto catchOp = currLexScope->getExceptionInfo().catchOp;
-  assert(catchOp && catchOp.getNumRegions() && "expected at least one region");
-  auto &fallbackRegion = catchOp.getRegion(catchOp.getNumRegions() - 1);
+  unsigned numCatchRegions = catchOp.getCatchRegions().size();
+  assert(catchOp && numCatchRegions && "expected at least one region");
+  auto &fallbackRegion = catchOp.getCatchRegions()[numCatchRegions - 1];
 
   auto *resumeBlock = &fallbackRegion.getBlocks().back();
   if (!resumeBlock->empty())
@@ -322,7 +323,6 @@ CIRGenFunction::buildCXXTryStmtUnderScope(const CXXTryStmt &S) {
 
   auto numHandlers = S.getNumHandlers();
   auto tryLoc = getLoc(S.getBeginLoc());
-  auto scopeLoc = getLoc(S.getSourceRange());
 
   mlir::OpBuilder::InsertPoint beginInsertTryBody;
   auto ehPtrTy = mlir::cir::PointerType::get(
@@ -335,28 +335,21 @@ CIRGenFunction::buildCXXTryStmtUnderScope(const CXXTryStmt &S) {
   // info but don't emit the bulk right away, for now only make sure the
   // scope returns the exception information.
   auto tryScope = builder.create<mlir::cir::TryOp>(
-      scopeLoc, /*scopeBuilder=*/
-      [&](mlir::OpBuilder &b, mlir::Type &yieldTy, mlir::Location loc) {
+      tryLoc, /*scopeBuilder=*/
+      [&](mlir::OpBuilder &b, mlir::Location loc) {
         // Allocate space for our exception info that might be passed down
         // to `cir.try_call` everytime a call happens.
-        yieldTy = ehPtrTy;
         exceptionInfoInsideTry = b.create<mlir::cir::AllocaOp>(
-            loc, /*addr type*/ getBuilder().getPointerTo(yieldTy),
-            /*var type*/ yieldTy, "__exception_ptr",
+            loc, /*addr type*/ getBuilder().getPointerTo(ehPtrTy),
+            /*var type*/ ehPtrTy, "__exception_ptr",
             CGM.getSize(CharUnits::One()), nullptr);
 
         beginInsertTryBody = getBuilder().saveInsertionPoint();
-      });
-
-  // The catch {} parts consume the exception information provided by a
-  // try scope. Also don't emit the code right away for catch clauses, for
-  // now create the regions and consume the try scope result.
-  // Note that clauses are later populated in
-  // CIRGenFunction::buildLandingPad.
-  auto catchOp = builder.create<mlir::cir::CatchOp>(
-      tryLoc,
-      tryScope->getResult(
-          0), // FIXME(cir): we can do better source location here.
+      },
+      // Don't emit the code right away for catch clauses, for
+      // now create the regions and consume the try scope result.
+      // Note that clauses are later populated in
+      // CIRGenFunction::buildLandingPad.
       [&](mlir::OpBuilder &b, mlir::Location loc,
           mlir::OperationState &result) {
         mlir::OpBuilder::InsertionGuard guard(b);
@@ -372,28 +365,24 @@ CIRGenFunction::buildCXXTryStmtUnderScope(const CXXTryStmt &S) {
 
   // Finally emit the body for try/catch.
   auto emitTryCatchBody = [&]() -> mlir::LogicalResult {
-    auto loc = catchOp.getLoc();
+    auto loc = tryScope.getLoc();
     mlir::OpBuilder::InsertionGuard guard(getBuilder());
     getBuilder().restoreInsertionPoint(beginInsertTryBody);
     CIRGenFunction::LexicalScope lexScope{*this, loc,
                                           getBuilder().getInsertionBlock()};
 
     {
-      lexScope.setExceptionInfo({exceptionInfoInsideTry, catchOp});
-      // Attach the basic blocks for the catchOp regions into ScopeCatch
-      // info.
-      enterCXXTryStmt(S, catchOp);
+      lexScope.setExceptionInfo({exceptionInfoInsideTry, tryScope});
+      // Attach the basic blocks for the catch regions.
+      enterCXXTryStmt(S, tryScope);
       // Emit the body for the `try {}` part.
       if (buildStmt(S.getTryBlock(), /*useCurrentScope=*/true).failed())
         return mlir::failure();
-
-      auto v = getBuilder().create<mlir::cir::LoadOp>(loc, ehPtrTy,
-                                                      exceptionInfoInsideTry);
-      getBuilder().create<mlir::cir::YieldOp>(loc, v.getResult());
+      getBuilder().create<mlir::cir::YieldOp>(loc);
     }
 
     {
-      lexScope.setExceptionInfo({tryScope->getResult(0), catchOp});
+      lexScope.setExceptionInfo({nullptr, tryScope});
       // Emit catch clauses.
       exitCXXTryStmt(S);
     }
@@ -452,7 +441,7 @@ static void buildCatchDispatchBlock(CIRGenFunction &CGF,
       // If the next handler is a catch-all, we're at the end, and the
       // next block is that handler.
     } else if (catchScope.getHandler(i + 1).isCatchAll()) {
-      // Block already created when creating CatchOp, just mark this
+      // Block already created when creating catch regions, just mark this
       // is the end.
       nextIsEnd = true;
     }
@@ -464,14 +453,14 @@ static void buildCatchDispatchBlock(CIRGenFunction &CGF,
 }
 
 void CIRGenFunction::enterCXXTryStmt(const CXXTryStmt &S,
-                                     mlir::cir::CatchOp catchOp,
+                                     mlir::cir::TryOp catchOp,
                                      bool IsFnTryBlock) {
   unsigned NumHandlers = S.getNumHandlers();
   EHCatchScope *CatchScope = EHStack.pushCatch(NumHandlers);
   for (unsigned I = 0; I != NumHandlers; ++I) {
     const CXXCatchStmt *C = S.getHandler(I);
 
-    mlir::Block *Handler = &catchOp.getRegion(I).getBlocks().front();
+    mlir::Block *Handler = &catchOp.getCatchRegions()[I].getBlocks().front();
     if (C->getExceptionDecl()) {
       // FIXME: Dropping the reference type on the type into makes it
       // impossible to correctly implement catch-by-reference
@@ -510,7 +499,18 @@ void CIRGenFunction::exitCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock) {
   if (!CatchScope.hasEHBranches()) {
     CatchScope.clearHandlerBlocks();
     EHStack.popCatch();
-    currLexScope->getExceptionInfo().catchOp->erase();
+    // Drop all basic block from all catch regions.
+    auto tryOp = currLexScope->getExceptionInfo().catchOp;
+    SmallVector<mlir::Block *> eraseBlocks;
+    for (mlir::Region &r : tryOp.getCatchRegions()) {
+      if (r.empty())
+        continue;
+      for (mlir::Block &b : r.getBlocks())
+        eraseBlocks.push_back(&b);
+    }
+    for (mlir::Block *b : eraseBlocks)
+      b->erase();
+    tryOp.setCatchTypesAttr({});
     return;
   }
 
@@ -630,43 +630,13 @@ mlir::Operation *CIRGenFunction::buildLandingPad() {
       return lpad;
   }
 
-  // If there's an existing CatchOp, it means we got a `cir.try` scope
+  // If there's an existing TryOp, it means we got a `cir.try` scope
   // that leads to this "landing pad" creation site. Otherwise, exceptions
-  // are enabled but a throwing function is called anyways.
-  auto catchOp = currLexScope->getExceptionInfo().catchOp;
-  if (!catchOp) {
-    auto loc = *currSrcLoc;
-    auto ehPtrTy = mlir::cir::PointerType::get(
-        getBuilder().getContext(),
-        getBuilder().getType<::mlir::cir::ExceptionInfoType>());
-
-    mlir::Value exceptionAddr;
-    {
-      // Get a new alloca within the current scope.
-      mlir::OpBuilder::InsertionGuard guard(builder);
-      exceptionAddr = buildAlloca(
-          "__exception_ptr", ehPtrTy, loc, CharUnits::One(),
-          builder.getBestAllocaInsertPoint(builder.getInsertionBlock()));
-    }
-
-    {
-      // Insert catch at the end of the block, and place the insert pointer
-      // back to where it was.
-      mlir::OpBuilder::InsertionGuard guard(builder);
-      auto exceptionPtr =
-          builder.create<mlir::cir::LoadOp>(loc, ehPtrTy, exceptionAddr);
-      catchOp = builder.create<mlir::cir::CatchOp>(
-          loc, exceptionPtr,
-          [&](mlir::OpBuilder &b, mlir::Location loc,
-              mlir::OperationState &result) {
-            // There's no source code level catch here, create one region for
-            // the resume block.
-            mlir::OpBuilder::InsertionGuard guard(b);
-            auto *r = result.addRegion();
-            builder.createBlock(r);
-          });
-    }
-    currLexScope->setExceptionInfo({exceptionAddr, catchOp});
+  // are enabled but a throwing function is called anyways (common pattern
+  // with function local static initializers).
+  auto tryOp = currLexScope->getExceptionInfo().catchOp;
+  if (!tryOp) {
+    llvm_unreachable("NYI");
   }
 
   {
@@ -752,17 +722,17 @@ mlir::Operation *CIRGenFunction::buildLandingPad() {
       assert(!MissingFeatures::setLandingPadCleanup());
     }
 
-    assert((clauses.size() > 0 || hasCleanup) && "CatchOp has no clauses!");
+    assert((clauses.size() > 0 || hasCleanup) && "no catch clauses!");
 
     // If there's no catch_all, attach the unwind region. This needs to be the
-    // last region in the CatchOp operation.
+    // last region in the TryOp operation catch list.
     if (!hasCatchAll) {
       auto catchUnwind = mlir::cir::CatchUnwindAttr::get(builder.getContext());
       clauses.push_back(catchUnwind);
     }
 
-    // Add final array of clauses into catchOp.
-    catchOp.setCatchersAttr(
+    // Add final array of clauses into TryOp.
+    tryOp.setCatchTypesAttr(
         mlir::ArrayAttr::get(builder.getContext(), clauses));
 
     // In traditional LLVM codegen. this tells the backend how to generate the
@@ -772,7 +742,7 @@ mlir::Operation *CIRGenFunction::buildLandingPad() {
     (void)getEHDispatchBlock(EHStack.getInnermostEHScope());
   }
 
-  return catchOp;
+  return tryOp;
 }
 
 // Differently from LLVM traditional codegen, there are no dispatch blocks

clang/lib/CIR/CodeGen/CIRGenFunction.h

@@ -316,7 +316,7 @@ class CIRGenFunction : public CIRGenTypeCache {
   /// allocas for the exception info
   struct CIRExceptionInfo {
     mlir::Value addr{};
-    mlir::cir::CatchOp catchOp{};
+    mlir::cir::TryOp catchOp{};
   };
 
   enum class EvaluationOrder {
@@ -940,7 +940,7 @@ class CIRGenFunction : public CIRGenTypeCache {
 
   mlir::LogicalResult buildCXXTryStmtUnderScope(const clang::CXXTryStmt &S);
   mlir::LogicalResult buildCXXTryStmt(const clang::CXXTryStmt &S);
-  void enterCXXTryStmt(const CXXTryStmt &S, mlir::cir::CatchOp catchOp,
+  void enterCXXTryStmt(const CXXTryStmt &S, mlir::cir::TryOp catchOp,
                        bool IsFnTryBlock = false);
   void exitCXXTryStmt(const CXXTryStmt &S, bool IsFnTryBlock = false);