[CIR] Add structured ResumeOp

[AmrDeveloper]

Add SCF Resume op to be used before the CFG flattening pass

Issue #154992

[llvmbot]

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Amr Hesham (AmrDeveloper)

Changes

Add SCF Resume op to be used before the CFG flattening pass

Issue #154992

---

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

2 Files Affected:

• (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+30)
• (modified) clang/test/CIR/IR/try-catch.cir (+22)

diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 5f5fab6f12300..0a8c06861a18b 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -898,6 +898,36 @@ def CIR_ContinueOp : CIR_Op<"continue", [Terminator]> {
   let hasLLVMLowering = false;
 }
 
+//===----------------------------------------------------------------------===//
+// Resume
+//===----------------------------------------------------------------------===//
+
+def CIR_SCFResumeOp : CIR_Op<"scf.resume", [
+  ReturnLike, Terminator
+]> {
+  let summary = "Resumes execution after not catching exceptions";
+  let description = [{
+    The `cir.scf.resume` operation handles an uncaught exception scenario.
+
+    Used as the terminator of a `CatchUnwind` region of `cir.try`, where it
+    does not receive any arguments (implied from the `cir.try` scope).
+
+    This operation is used only before the CFG flatterning pass.
+
+    Examples:
+    ```mlir
+    cir.try {
+      cir.yield
+    } unwind {
+      cir.scf.resume
+    }
+    ```
+  }];
+
+  let assemblyFormat = "attr-dict";
+  let hasLLVMLowering = false;
+}
+
 //===----------------------------------------------------------------------===//
 // ScopeOp
 //===----------------------------------------------------------------------===//
diff --git a/clang/test/CIR/IR/try-catch.cir b/clang/test/CIR/IR/try-catch.cir
index 7becd0b559f5e..44263345caee3 100644
--- a/clang/test/CIR/IR/try-catch.cir
+++ b/clang/test/CIR/IR/try-catch.cir
@@ -81,4 +81,26 @@ cir.func dso_local @empty_try_block_with_catch_ist() {
 // CHECK:   cir.return
 // CHECK: }
 
+cir.func dso_local @empty_try_block_with_catch_unwind_contains_resume() {
+  cir.scope {
+    cir.try {
+      cir.yield
+    } unwind {
+      cir.scf.resume
+    }
+  }
+  cir.return
+}
+
+// CHECK: cir.func dso_local @empty_try_block_with_catch_unwind_contains_resume() {
+// CHECK:   cir.scope {
+// CHECK:     cir.try {
+// CHECK:       cir.yield
+// CHECK:     } unwind {
+// CHECK:       cir.scf.resume
+// CHECK:     }
+// CHECK:   }
+// CHECK:   cir.return
+// CHECK: }
+
 }

[andykaylor]

Suggested change

	def CIR_SCFResumeOp : CIR_Op<"scf.resume", [
	def CIR_SCFResumeOp : CIR_Op<"structured.resume", [

I don't really like the use of scf because it's a dialect name. @xlauko What do you think?

[xlauko]

I would argue everybode else knows what scf is, so it is more clear what cir.scf refers to.
I percieved this as our scf subdialect, i.e. cir::scf (sadly MLIR does not have a concept of subdialects) .
We do the same for cir.complex for instance.

[xlauko]

I would also suggest to rename this to cir.scf.unwind_resume as cir.scf.resume is a bit ambiguous. And without description I did not guessed this is just meant for unwind region.

[xlauko]

Maybe also add HasParent trait.

[xlauko]

Or cir.scf.rethrow​? @andykaylor @bcardosolopes what do you think?

[AmrDeveloper]

In the incubator, we have one ResumeOp that has 2 modes (Similar to CatchParamOp).

First mode in the unwind block

cir.try {

} unwind {
   cir.resume <----
}

And the second mode after the CFG flattening

^bb(%exception: ...., %type_id: .....)
  cir.resume %exception, %type_id <----- 

In FlattenCFG, we replace ResumeOp with another ResumeOp, but we fill the arguments in the new Op

rewriter.replaceOpWithNewOp<cir::ResumeOp>(
        resume, unwindBlock->getArgument(0), unwindBlock->getArgument(1));

• In the first mode, I don't think naming it unwind_resume will add more context because it's only used in the unwind block and there no other resume.
• We need to define a pattern for Op that was used in 2 modes and is now converted into 2 ops. Either we add a prefix for the structured one or the other one, so if we renamed this one cir.resume, that means we will rename the non-SCF ops with a prefix? 🤔 because we will face the same question in CatchParamOp too.

What do you think?

[andykaylor]

There is some precedent set by SwitchFlatOp (cir.switch.flat), which is the flattened form of SwitchOp. How about cir.resume for the structured version and cir.resume.flat after flattening?

[AmrDeveloper]

There is some precedent set by SwitchFlatOp (cir.switch.flat), which is the flattened form of SwitchOp. How about cir.resume for the structured version and cir.resume.flat after flattening?

I think that's good, and also to be consistent

[xlauko]

Not really a fan of .flat suffix, but I guess we can refactor structure/unstrucured control flow together once more of it is upstreamed :)

[AmrDeveloper]

I guess we can refactor structure/unstrucured control flow together once more of it is upstreamed :)

Nice, I think after CatchParam we can discuss that and refactor them 👍🏻

[xlauko]

I would also suggest to rename this to cir.scf.unwind_resume as cir.scf.resume is a bit ambiguous. And without description I did not guessed this is just meant for unwind region.

[xlauko]

Maybe also add HasParent trait.

[bcardosolopes]

LGTM

[bcardosolopes]

I prefer the simple 'cir.resume', 'cir.eh_resume', 'cir.unwind_resume'. I'd avoid giving any expectations to users of what scf might mean and would keep it out.

[andykaylor]

lgtm