[CIR] Upstream CIR Dialect TryOp with Catch Attrs

[AmrDeveloper]

Upstream CIR TryOp with catch attributes as a prerequisite for implementing try-catch in #162528

Issue #154992

[llvmbot]

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Amr Hesham (AmrDeveloper)

Changes

Upstream CIR TryOp with catch attributes

Issue #154992

---

Full diff: https://github.com/llvm/llvm-project/pull/162897.diff

4 Files Affected:

• (modified) clang/include/clang/CIR/Dialect/IR/CIRAttrs.td (+15)
• (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+61-2)
• (modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+132)
• (added) clang/test/CIR/IR/try-catch.cir (+84)

diff --git a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
index bb62223d9e152..38b53396bad31 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIRAttrs.td
@@ -960,4 +960,19 @@ def CIR_TypeInfoAttr : CIR_Attr<"TypeInfo", "typeinfo", [TypedAttrInterface]> {
   }];
 }
 
+//===----------------------------------------------------------------------===//
+// CatchAllAttr & CatchUnwindAttr
+//===----------------------------------------------------------------------===//
+
+// Represents the unwind region where unwind continues or
+// the program std::terminate's.
+def CIR_CatchUnwindAttr : CIR_UnitAttr<"CatchUnwind", "unwind"> {
+  let storageType = [{ CatchUnwind }];
+}
+
+// Represents the catch_all region.
+def CIR_CatchAllAttr : CIR_UnitAttr<"CatchAll", "all"> {
+  let storageType = [{ CatchAllAttr }];
+}
+
 #endif // CLANG_CIR_DIALECT_IR_CIRATTRS_TD
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 27fe0cc46d7cf..1171b6f820341 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -631,7 +631,7 @@ def CIR_StoreOp : CIR_Op<"store", [
 
 defvar CIR_ReturnableScopes = [
   "FuncOp", "ScopeOp", "IfOp", "SwitchOp", "CaseOp",
-  "DoWhileOp", "WhileOp", "ForOp"
+  "DoWhileOp", "WhileOp", "ForOp", "TryOp"
 ];
 
 def CIR_ReturnOp : CIR_Op<"return", [
@@ -778,7 +778,7 @@ def CIR_ConditionOp : CIR_Op<"condition", [
 
 defvar CIR_YieldableScopes = [
   "ArrayCtor", "ArrayDtor", "CaseOp", "DoWhileOp", "ForOp", "GlobalOp", "IfOp",
-  "ScopeOp", "SwitchOp", "TernaryOp", "WhileOp"
+  "ScopeOp", "SwitchOp", "TernaryOp", "WhileOp", "TryOp"
 ];
 
 def CIR_YieldOp : CIR_Op<"yield", [
@@ -4296,6 +4296,65 @@ def CIR_AllocExceptionOp : CIR_Op<"alloc.exception"> {
   }];
 }
 
+//===----------------------------------------------------------------------===//
+// TryOp
+//===----------------------------------------------------------------------===//
+
+def CIR_TryOp : CIR_Op<"try",[
+  DeclareOpInterfaceMethods<RegionBranchOpInterface>,
+  RecursivelySpeculatable, AutomaticAllocationScope, NoRegionArguments
+]> {
+  let summary = "C++ try block";
+  let description = [{
+    Holds the lexical scope of `try {}`. Note that resources used on catch
+    clauses are usually allocated in the same parent as `cir.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:
+
+    ```mlir
+    %0 = cir.alloc.exception 16 -> !cir.ptr<!some_record>
+    %1 = cir.get_global @d2 : !cir.ptr<!some_record>
+    cir.try synthetic cleanup {
+      cir.call exception @_ZN7test2_DC1ERKS_(%0, %1)
+            : (!cir.ptr<!some_record>, !cir.ptr<!some_record>) -> () cleanup {
+        %2 = cir.cast bitcast %0 : !cir.ptr<!some_record> -> !cir.ptr<!void>
+        cir.free.exception %2
+        cir.yield
+      }
+      ...
+    }
+    ```
+  }];
+
+  let arguments = (ins UnitAttr:$synthetic, UnitAttr:$cleanup,
+                       OptionalAttr<ArrayAttr>:$catch_types);
+  let regions = (region AnyRegion:$try_region,
+                        VariadicRegion<AnyRegion>:$catch_regions);
+
+  let assemblyFormat = [{
+    (`synthetic` $synthetic^)?
+    (`cleanup` $cleanup^)?
+    $try_region
+    custom<CatchRegions>($catch_regions, $catch_types)
+    attr-dict
+  }];
+
+  // Everything already covered elsewhere.
+  let builders = [OpBuilder<(ins
+      "llvm::function_ref<void(mlir::OpBuilder &, "
+        "mlir::Location)>":$tryBuilder,
+      "llvm::function_ref<void(mlir::OpBuilder &, mlir::Location, "
+        "mlir::OperationState &)>":$catchBuilder)>];
+
+  let hasLLVMLowering = false;
+}
+
 //===----------------------------------------------------------------------===//
 // Atomic operations
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index 5f88590c48d30..6e6eb4e5c9093 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -2878,6 +2878,138 @@ LogicalResult cir::TypeInfoAttr::verify(
   return success();
 }
 
+//===----------------------------------------------------------------------===//
+// TryOp
+//===----------------------------------------------------------------------===//
+
+void cir::TryOp::build(
+    OpBuilder &builder, OperationState &result,
+    function_ref<void(OpBuilder &, Location)> tryBuilder,
+    function_ref<void(OpBuilder &, Location, OperationState &)> catchBuilder) {
+  assert(tryBuilder && "expected builder callback for 'cir.try' body");
+  assert(catchBuilder && "expected builder callback for 'catch' body");
+
+  OpBuilder::InsertionGuard guard(builder);
+
+  // Try body region
+  Region *tryBodyRegion = result.addRegion();
+
+  // Create try body region and set insertion point
+  builder.createBlock(tryBodyRegion);
+  tryBuilder(builder, result.location);
+  catchBuilder(builder, result.location, result);
+}
+
+void cir::TryOp::getSuccessorRegions(
+    mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> &regions) {
+  // If any index all the underlying regions branch back to the parent
+  // operation.
+  if (!point.isParent()) {
+    regions.push_back(RegionSuccessor());
+    return;
+  }
+
+  // If the condition isn't constant, both regions may be executed.
+  regions.push_back(RegionSuccessor(&getTryRegion()));
+
+  // FIXME: optimize, ideas include:
+  // - If we know a target function never throws a specific type, we can
+  //   remove the catch handler.
+  for (mlir::Region &r : this->getCatchRegions())
+    regions.push_back(RegionSuccessor(&r));
+}
+
+static void printCatchRegions(OpAsmPrinter &printer, cir::TryOp op,
+                              mlir::MutableArrayRef<::mlir::Region> regions,
+                              mlir::ArrayAttr catchersAttr) {
+  if (!catchersAttr)
+    return;
+
+  int currCatchIdx = 0;
+  printer << "catch [";
+  llvm::interleaveComma(catchersAttr, printer, [&](const Attribute &a) {
+    if (mlir::isa<cir::CatchUnwindAttr>(a)) {
+      printer.printAttribute(a);
+      printer << " ";
+    } else if (!a) {
+      printer << "all";
+    } else {
+      printer << "type ";
+      printer.printAttribute(a);
+      printer << " ";
+    }
+    printer.printRegion(regions[currCatchIdx], /*printEntryBLockArgs=*/false,
+                        /*printBlockTerminators=*/true);
+    currCatchIdx++;
+  });
+
+  printer << "]";
+}
+
+static ParseResult parseCatchRegions(
+    OpAsmParser &parser,
+    llvm::SmallVectorImpl<std::unique_ptr<::mlir::Region>> &regions,
+    ::mlir::ArrayAttr &catchersAttr) {
+  if (parser.parseKeyword("catch").failed())
+    return parser.emitError(parser.getCurrentLocation(),
+                            "expected 'catch' keyword here");
+
+  auto parseAndCheckRegion = [&]() -> ParseResult {
+    // Parse region attached to catch
+    regions.emplace_back(new Region);
+    Region &currRegion = *regions.back();
+    SMLoc parserLoc = parser.getCurrentLocation();
+    if (parser.parseRegion(currRegion)) {
+      regions.clear();
+      return failure();
+    }
+
+    if (currRegion.empty()) {
+      return parser.emitError(parser.getCurrentLocation(),
+                              "catch region shall not be empty");
+    }
+
+    if (!(currRegion.back().mightHaveTerminator() &&
+          currRegion.back().getTerminator()))
+      return parser.emitError(
+          parserLoc, "blocks are expected to be explicitly terminated");
+
+    return success();
+  };
+
+  llvm::SmallVector<mlir::Attribute, 4> catchList;
+  auto parseCatchEntry = [&]() -> ParseResult {
+    mlir::Attribute exceptionTypeInfo;
+
+    if (parser.parseOptionalAttribute(exceptionTypeInfo).has_value()) {
+      catchList.push_back(exceptionTypeInfo);
+    } else {
+      ::llvm::StringRef attrStr;
+      if (parser.parseOptionalKeyword(&attrStr, {"all"}).succeeded()) {
+        // "all" keyword found, exceptionTypeInfo remains null
+      } else if (parser.parseOptionalKeyword("type").succeeded()) {
+        if (parser.parseAttribute(exceptionTypeInfo).failed())
+          return parser.emitError(parser.getCurrentLocation(),
+                                  "expected valid RTTI info attribute");
+      } else {
+        return parser.emitError(parser.getCurrentLocation(),
+                                "expected attribute, 'all', or 'type' keyword");
+      }
+      catchList.push_back(exceptionTypeInfo);
+    }
+    return parseAndCheckRegion();
+  };
+
+  if (parser
+          .parseCommaSeparatedList(OpAsmParser::Delimiter::Square,
+                                   parseCatchEntry, " in catch list")
+          .failed())
+    return failure();
+
+  catchersAttr = parser.getBuilder().getArrayAttr(catchList);
+  return ::mlir::success();
+}
+
 //===----------------------------------------------------------------------===//
 // TableGen'd op method definitions
 //===----------------------------------------------------------------------===//
diff --git a/clang/test/CIR/IR/try-catch.cir b/clang/test/CIR/IR/try-catch.cir
new file mode 100644
index 0000000000000..7bc71ff84d4ae
--- /dev/null
+++ b/clang/test/CIR/IR/try-catch.cir
@@ -0,0 +1,84 @@
+// RUN: cir-opt %s --verify-roundtrip | FileCheck %s
+
+!u8i = !cir.int<u, 8>
+
+module {
+
+cir.global "private" constant external @_ZTIi : !cir.ptr<!u8i>
+cir.global "private" constant external @_ZTIPKc : !cir.ptr<!u8i>
+
+cir.func dso_local @empty_try_block_with_catch_all() {
+  cir.scope {
+    cir.try {
+      cir.yield
+    } catch [type #cir.all {
+      cir.yield
+    }]
+  }
+  cir.return
+}
+
+// CHECK:  cir.func dso_local @empty_try_block_with_catch_all() {
+// CHECK:    cir.scope {
+// CHECK:      cir.try {
+// CHECK:        cir.yield
+// CHECK:      } catch [type #cir.all {
+// CHECK:        cir.yield
+// CHECK:      }]
+// CHECK:    }
+// CHECK:    cir.return
+// CHECK:  }
+
+cir.func dso_local @empty_try_block_with_catch_unwind() {
+  cir.scope {
+    cir.try {
+      cir.yield
+    } catch [#cir.unwind {
+      cir.yield
+    }]
+  }
+  cir.return
+}
+
+// CHECK: cir.func dso_local @empty_try_block_with_catch_unwind() {
+// CHECK:  cir.scope {
+// CHECK:    cir.try {
+// CHECK:      cir.yield
+// CHECK:    } catch [#cir.unwind {
+// CHECK:      cir.yield
+// CHECK:    }]
+// CHECK:  }
+// CHECK:  cir.return
+// CHECK: }
+
+cir.func dso_local @empty_try_block_with_catch_ist() {
+  cir.scope {
+    cir.try {
+      cir.yield
+    } catch [type #cir.global_view<@_ZTIi> : !cir.ptr<!u8i> {
+      cir.yield
+    }, type #cir.global_view<@_ZTIPKc> : !cir.ptr<!u8i> {
+      cir.yield
+    }, #cir.unwind {
+      cir.yield
+    }]
+  }
+  cir.return
+}
+
+// CHECK: cir.func dso_local @empty_try_block_with_catch_ist() {
+// CHECK:   cir.scope {
+// CHECK:     cir.try {
+// CHECK:       cir.yield
+// CHECK:     } catch [type #cir.global_view<@_ZTIi> : !cir.ptr<!u8i> {
+// CHECK:       cir.yield
+// CHECK:     }, type #cir.global_view<@_ZTIPKc> : !cir.ptr<!u8i> {
+// CHECK:       cir.yield
+// CHECK:     }, #cir.unwind {
+// CHECK:       cir.yield
+// CHECK:     }]
+// CHECK:   }
+// CHECK:   cir.return
+// CHECK: }
+
+}

[bcardosolopes]

Is this part of splitting off #162528? Might be handy to mention these things in the description.

[AmrDeveloper]

handy

Sure, That makes sense

[bcardosolopes]

Thanks for splitting this up!

[andykaylor]

Is this a catch region?

[AmrDeveloper]

Yes, because the current format is like a followed by an array of catcher's blocks, I agree it would be better to change the format to have catch before each block

[andykaylor]

This should be described in terms of the semantics of the CIR itself.

Suggested change

	`cleanup`: signal to targets (LLVM for now) that this try/catch, needs
	to specially tag their landing pads as needing "cleanup".
	`cleanup`: indicates that there are cleanups that must be performed
	when exiting the try region via exception, even if the exception is not
	caught.

[andykaylor]

Suggested change

	present in the source code (e.g. `g = new Class` under `-fexceptions`).
	present in the source code. For example, a synthetic `cir.try` region
	is created around the constructor call when `operator new` is used
	so that the memory allocated will be freed if the constructor throws
	an exception.

[andykaylor]

Shouldn't there be an unwind region here?

[andykaylor]

I don't like the square brackets around the catch region. Is there some precedent for that in other dialects?

Currently we have something that looks like this in the incubator:

      cir.try {
        cir.call exception @_Z3foov() : () -> ()
        cir.yield
      } catch [type #cir.global_view<@_ZTIPi> : !cir.ptr<!u8i> {
        %1 = cir.catch_param -> !cir.ptr<!s32i>
        cir.store align(8) %1, %0 : !cir.ptr<!s32i>, !cir.ptr<!cir.ptr<!s32i>>
        cir.yield
      }, type #cir.global_view<@_ZTIPf> : !cir.ptr<!u8i> {
        %2 = cir.catch_param -> !cir.ptr<!cir.float>
        cir.store align(8) %2, %0 : !cir.ptr<!cir.float>, !cir.ptr<!cir.ptr<!cir.float>>
        cir.yield
      }, #cir.unwind {
        cir.resume
      }]

I'd prefer to see something like this:

      cir.try {
        cir.call exception @_Z3foov() : () -> ()
        cir.yield
      } catch [type #cir.global_view<@_ZTIPi> : !cir.ptr<!u8i>] {
        %1 = cir.catch_param -> !cir.ptr<!s32i>
        cir.store align(8) %1, %0 : !cir.ptr<!s32i>, !cir.ptr<!cir.ptr<!s32i>>
        cir.yield
      } catch [type #cir.global_view<@_ZTIPf> : !cir.ptr<!u8i>] {
        %2 = cir.catch_param -> !cir.ptr<!cir.float>
        cir.store align(8) %2, %0 : !cir.ptr<!cir.float>, !cir.ptr<!cir.ptr<!cir.float>>
        cir.yield
      } unwind {
        cir.resume
      }

[AmrDeveloper]

That looks much better, for a catch all, I think we don't need to print all as an attribute. Maybe we can print it like this? 🤔

Before

} catch [#cir.all] {
}

After

} catch all {
}

[andykaylor]

Shouldn't this be handled by a CatchAllAttr?

[andykaylor]

What is this parsing? It looks like it would handle any kind of attribute.

[andykaylor]

This should only accept a global view attribute referencing type info or the catch-all attribute (which I'd prefer to see handled differently). I think the code as written would accept any valid attribute.

[andykaylor]

This seems redundant.