[CIR] Add flattened version of CatchParamOp #172713
Conversation
AmrDeveloper
requested review from
andykaylor,
bcardosolopes,
lanza and
xlauko
as code owners
llvmbot
added
clang
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Amr Hesham (AmrDeveloper) ChangesAdd flattened version of CatchParamOp Issue #154992 Full diff: https://github.com/llvm/llvm-project/pull/172713.diff 4 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 0e91d008dc52d..d26afca3efc64 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -5366,6 +5366,48 @@ def CIR_CatchParamOp : CIR_Op<"catch_param", [HasParent<"cir::TryOp">]> {
let hasLLVMLowering = false;
}
+//===----------------------------------------------------------------------===//
+// CatchParamFlatOp
+//===----------------------------------------------------------------------===//
+
+def CIR_FlatCatchParamKind : CIR_I32EnumAttr<
+ "FlatCatchParamKind", "Designate limits for begin/end of catch param handling", [
+ I32EnumAttrCase<"Begin", 0, "begin">,
+ I32EnumAttrCase<"End", 1, "end">
+]>;
+
+def CIR_CatchParamFlatOp : CIR_Op<"catch_param.flat"> {
+ let summary = "A flattened version of `cir.catch_param`";
+ let description = [{
+ The `cir.catch_param.flat` operation is a region-less and simplified
+ version of the `cir.catch_param`.
+
+ Its representation is closer to LLVM IR dialect
+ than the C/C++ language feature.
+
+ This operation is used only after the CFG flatterning pass.
+
+ Examples:
+ ```mlir
+ cir.catch_param.flat begin %exception_ptr -> !cir.ptr<!s32i>
+ cir.catch_param.flat end
+ ```
+ }];
+
+ let arguments = (ins
+ Optional<CIR_VoidPtrType>:$exception_ptr,
+ CIR_FlatCatchParamKind:$kind
+ );
+
+ let results = (outs Optional<CIR_AnyType>:$param);
+ let assemblyFormat = [{
+ $kind
+ ($exception_ptr^)?
+ (`->` qualified(type($param))^)?
+ attr-dict
+ }];
+}
+
//===----------------------------------------------------------------------===//
// Exception related: EhInflightOp
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 7c9cf8e2c2e2d..c28915cd04f53 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -3418,6 +3418,30 @@ mlir::LogicalResult CIRToLLVMEhInflightOpLowering::matchAndRewrite(
return mlir::success();
}
+mlir::LogicalResult CIRToLLVMCatchParamFlatOpLowering::matchAndRewrite(
+ cir::CatchParamFlatOp op, OpAdaptor adaptor,
+ mlir::ConversionPatternRewriter &rewriter) const {
+ if (op.getKind() == cir::FlatCatchParamKind::Begin) {
+ StringRef fnName = "__cxa_begin_catch";
+ auto llvmPtrTy = mlir::LLVM::LLVMPointerType::get(rewriter.getContext());
+ auto fnTy = mlir::LLVM::LLVMFunctionType::get(llvmPtrTy, {llvmPtrTy});
+ createLLVMFuncOpIfNotExist(rewriter, op, fnName, fnTy);
+ rewriter.replaceOpWithNewOp<mlir::LLVM::CallOp>(
+ op, mlir::TypeRange{llvmPtrTy}, fnName,
+ mlir::ValueRange{adaptor.getExceptionPtr()});
+ return mlir::success();
+ }
+
+ assert(op.getKind() == cir::FlatCatchParamKind::End);
+ StringRef fnName = "__cxa_end_catch";
+ auto fnTy = mlir::LLVM::LLVMFunctionType::get(
+ mlir::LLVM::LLVMVoidType::get(rewriter.getContext()), {});
+ createLLVMFuncOpIfNotExist(rewriter, op, fnName, fnTy);
+ rewriter.replaceOpWithNewOp<mlir::LLVM::CallOp>(op, mlir::TypeRange{}, fnName,
+ mlir::ValueRange{});
+ return mlir::success();
+}
+
mlir::LogicalResult CIRToLLVMTrapOpLowering::matchAndRewrite(
cir::TrapOp op, OpAdaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const {
diff --git a/clang/test/CIR/IR/catch-param-flat.cir b/clang/test/CIR/IR/catch-param-flat.cir
new file mode 100644
index 0000000000000..3a3b10a25c216
--- /dev/null
+++ b/clang/test/CIR/IR/catch-param-flat.cir
@@ -0,0 +1,22 @@
+// RUN: cir-opt %s --verify-roundtrip | FileCheck %s
+
+!s32i = !cir.int<s, 32>
+!void = !cir.void
+
+module {
+
+cir.func dso_local @catch_param_flat() personality(@__gxx_personality_v0) {
+ %exception_ptr, %type_id = cir.eh.inflight_exception
+ %exception = cir.catch_param.flat begin %exception_ptr -> !cir.ptr<!s32i>
+ cir.catch_param.flat end
+ cir.return
+}
+
+// CHECK: cir.func dso_local @catch_param_flat() personality(@__gxx_personality_v0) {
+// CHECK: %[[EXCEPTION_PTR:.*]], %[[TYPE_ID:.*]] = cir.eh.inflight_exception
+// CHECK: %[[EXCEPTION:.*]] = cir.catch_param.flat begin %[[EXCEPTION_PTR]] -> !cir.ptr<!s32i>
+// CHECK: cir.catch_param.flat end
+// CHECK: cir.return
+// CHECK: }
+
+}
diff --git a/clang/test/CIR/Lowering/catch-param-flat.cir b/clang/test/CIR/Lowering/catch-param-flat.cir
new file mode 100644
index 0000000000000..a7307d7cf98eb
--- /dev/null
+++ b/clang/test/CIR/Lowering/catch-param-flat.cir
@@ -0,0 +1,25 @@
+// RUN: cir-opt %s -cir-to-llvm -o %t.cir
+
+!s32i = !cir.int<s, 32>
+!void = !cir.void
+
+module {
+
+cir.func dso_local @catch_param_flat() personality(@__gxx_personality_v0) {
+ %exception_ptr, %type_id = cir.eh.inflight_exception
+ %exception = cir.catch_param.flat begin %exception_ptr -> !cir.ptr<!s32i>
+ cir.catch_param.flat end
+ cir.return
+}
+
+// CHECK: llvm.func @catch_param_flat() attributes {dso_local, personality = @__gxx_personality_v0} {
+// CHECK: %[[CONST_0:.*]] = llvm.mlir.zero : !llvm.ptr
+// CHECK: %[[LANDING_PAD:.*]] = llvm.landingpad (catch %[[CONST_0]] : !llvm.ptr) : !llvm.struct<(ptr, i32)>
+// CHECK: %[[EXCEPTION_PTR:.*]] = llvm.extractvalue %[[LANDING_PAD]][0] : !llvm.struct<(ptr, i32)>
+// CHECK: %[[TYPE_ID:.*]] = llvm.extractvalue %[[LANDING_PAD]][1] : !llvm.struct<(ptr, i32)>
+// CHECK: %[[EXCEPTION:.*]] = llvm.call @__cxa_begin_catch(%[[EXCEPTION_PTR]]) : (!llvm.ptr) -> !llvm.ptr
+// CHECK: llvm.call @__cxa_end_catch() : () -> ()
+// CHECK: llvm.return
+// CHECK: }
+
+}
|
andykaylor
left a comment
andykaylor
left a comment
There was a problem hiding this comment.
I apologize for not having raised this issue sooner, but I'm not sure how well our current set of instructions will lend itself to Windows support. Take a simple case like this:
int bar() {
int ret = 0;
try {
foo();
} catch (int n) {
ret = n;
}
return ret;
}
We'll generate a catch region that looks something like this in CIR after flattening:
^bb4: // pred: ^bb2
%exception_ptr, %type_id = cir.eh.inflight_exception [@_ZTIi]
cir.br ^bb5(%exception_ptr, %type_id : !cir.ptr<!void>, !u32i)
^bb5(%4: !cir.ptr<!void>, %5: !u32i): // pred: ^bb4
%6 = cir.eh.typeid @_ZTIi
%7 = cir.cmp(eq, %5, %6) : !u32i, !cir.bool
cir.brcond %7 ^bb6(%4 : !cir.ptr<!void>), ^bb7(%4, %5 : !cir.ptr<!void>, !u32i)
^bb6(%8: !cir.ptr<!void>): // pred: ^bb5
%9 = cir.catch_param.flat begin %8 -> !cir.ptr<!s32i>
%10 = cir.load align(4) %9 : !cir.ptr<!s32i>, !s32i
cir.store align(4) %10, %0 : !s32i, !cir.ptr<!s32i>
%11 = cir.load align(4) %0 : !cir.ptr<!s32i>, !s32i
cir.store align(4) %11, %2 : !s32i, !cir.ptr<!s32i>
cir.catch_param.flat end
cir.br ^bb8
^bb7(%12: !cir.ptr<!void>, %13: !u32i): // pred: ^bb5
cir.resume.flat %12, %13
On Linux, we need that to result in LLVM IR that looks like this:
7:
%8 = landingpad { ptr, i32 }
catch ptr @_ZTIi, !dbg !15
%9 = extractvalue { ptr, i32 } %8, 0
%10 = extractvalue { ptr, i32 } %8, 1
br label %11
11:
%12 = phi ptr [ %9, %7 ]
%13 = phi i32 [ %10, %7 ]
%14 = call i32 @llvm.eh.typeid.for.p0(ptr @_ZTIi)
%15 = icmp eq i32 %13, %14
br i1 %15, label %16, label %21
16:
%17 = phi ptr [ %12, %11 ]
%18 = call ptr @__cxa_begin_catch(ptr %17)
%19 = load i32, ptr %18
store i32 %19, ptr %1
%20 = load i32, ptr %1
store i32 %20, ptr %3
call void @__cxa_end_catch()
br label %26
So far, so good, because cir.eh.inflight_exception will get translated to the landingpad instruction, cir.catch_param.flat begin will get translated to __cxa_begin_catch and cir.catch_param.flat end will get translated to __cxa_end_catch.
But on Windows, we need to be able to lower to LLVM IR that looks like this:
catch.dispatch:
%0 = catchswitch within none [label %catch] unwind to caller
catch:
%1 = catchpad within %0 [ptr @"??_R0H@8", i32 0, ptr %n]
#dbg_declare(ptr %n, !20
%2 = load i32, ptr %n
store i32 %2, ptr %ret
catchret from %1 to label %catchret.dest
catchret.dest:
br label %try.cont, !dbg !24
At this point, I think it's clear that our CIR constructs are too closely tied to the Itanium exception handling ABI. The whole business with getting a type id as a return value from cir.eh.inflight_exception and comparing it the type id we're catching is an ABI-specific detail.
We need to step back and redesign our operations to be more ABI-neutral. I propose the following (using the example above):
^bb4: // pred: ^bb2
%eh_token = cir.eh.landingpad
// Dispatch to handlers based on type id globals, with behavior similar to cir.switch.flat
// ^bb7 is the default destination if none of our catch handler match
cir.eh.dispatch %eh_token : !cir.eh_token, ^bb7 [
@_ZTIi : ^bb6
]
^bb6(%8 : !cir.eh_token): // pred: ^bb5
%catch_token, %exception_ptr = cir.begin_catch %8 -> !cir.ptr<!s32i>
%10 = cir.load align(4) %exception_ptr : !cir.ptr<!s32i>, !s32i
cir.store align(4) %10, %0 : !s32i, !cir.ptr<!s32i>
%11 = cir.load align(4) %0 : !cir.ptr<!s32i>, !s32i
cir.store align(4) %11, %2 : !s32i, !cir.ptr<!s32i>
cir.end_catch_param %catch_token
cir.br ^bb8
^bb7(%12: !cir.ptr<!void>, %13: !u32i): // pred: ^bb5
cir.resume.flat %12, %13
My thinking here is that cir.eh.landingpad would be expanded to a series of blocks containing landingpad and the subsequent branching for Itanium, while on Windows it would create a cleanuppad if necessary but otherwise would just be used to track the funclet token. Then
cir.eh.dispatch would be lowered to the appropriate type id comparison and branches for Itanium or a catchswitch for Windows. Finally, cir.begin_catch/cir.end_catch lower to __cxa_catch_begin/__cxa_end_catch for Itanim and to catchpad/catchret for Windows.
There's another complication on Windows when you have a try-block inside a catch because then we need to pass the funclet token through invoke instructions, but I think we can work out the details of that later. At this point, I just want to make sure we aren't going in a direction that won't work at all for Windows.
@rnk did a lot of work on the existing exception handling and offered to help with reviews as we rework this, so I'm going to throw him in the deep end by asking for his input here. 😇
|
|
||
| Examples: | ||
| ```mlir | ||
| %exception = cir.catch_param.flat begin %exception_ptr -> !cir.ptr<!s32i> |
There was a problem hiding this comment.
Would it make more sense if we just introduced separate cir.begin_catch and cir,.end_catch operations? It's not clear to me in what sense cir.catch_param.flat end is really associated with the catch parameter.
There was a problem hiding this comment.
I think yes, especially after the redesign it will be better to split them into two operations
This is a very good point, and I think you mentioned it before that we need to check about the catch param for windows, I will also check what the original codegen does for windows to understand more :D |
If i understood correctly cir.eh_token in Itanium will be lowered to a struct |
The token is just an opaque value to establish a use-def chain. When the operation is lowered for Itanium, the replacement will be as you describe, though I wouldn't necessarily think of it in term of the token become the struct. For Windows, the token roughly corresponds to the funclet token, meaning it gets lowered to |
That make sense, thanks |
3 participants
Add flattened version of CatchParamOp
Issue #154992