Address code review comments

AmrDeveloper · AmrDeveloper · commit 94e8d43fd60f · 2025-11-16T14:55:46.000+01:00
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -4615,22 +4615,31 @@ def CIR_EhInflightOp : CIR_Op<"eh.inflight_exception"> {
   let description = [{
     `cir.eh.inflight_exception` returns two values:
       - `exception_ptr`: The exception pointer for the inflight exception
-      - `type_id`: pointer to the exception object
-    This operation is expected to be the first one basic blocks on the
-    exception path out of `cir.try_call` operations.
+      - `type_id`: the type info index for the exception type
+    This operation is expected to be the first operation in the unwind
+    destination basic blocks of a `cir.try_call` operation.
 
-    The `cleanup` attribute indicates that clean up code might run before the
-    values produced by this operation are used to gather exception information.
+    The `cleanup` attribute indicates that clean up code must be run before the
+    values produced by this operation are used to dispatch the exception. This
+    cleanup code must be executed even if the exception is not caught.
     This helps CIR to pass down more accurate information for LLVM lowering
     to landingpads.
+
+    Example:
+
+    ```mlir
+    %exception_ptr, %type_id = cir.eh.inflight_exception
+    %exception_ptr, %type_id = cir.eh.inflight_exception [@_ZTIi, @_ZTIPKc]
+    %exception_ptr, %type_id = cir.eh.inflight_exception cleanup
+    ``
   }];
 
   let arguments = (ins UnitAttr:$cleanup,
-                       OptionalAttr<FlatSymbolRefArrayAttr>:$sym_type_list);
+                       OptionalAttr<FlatSymbolRefArrayAttr>:$catch_type_list);
   let results = (outs CIR_VoidPtrType:$exception_ptr, CIR_UInt32:$type_id);
   let assemblyFormat = [{
     (`cleanup` $cleanup^)?
-    ($sym_type_list^)?
+    ($catch_type_list^)?
     attr-dict
   }];
 }
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -3082,42 +3082,44 @@ mlir::LogicalResult CIRToLLVMEhInflightOpLowering::matchAndRewrite(
   mlir::Block *entryBlock = &llvmFn.getRegion().front();
   assert(entryBlock->isEntryBlock());
 
-  mlir::ArrayAttr symListAttr = op.getSymTypeListAttr();
-  mlir::SmallVector<mlir::Value, 4> symAddrs;
+  mlir::ArrayAttr catchListAttr = op.getCatchTypeListAttr();
+  mlir::SmallVector<mlir::Value> catchSymAddrs;
 
   auto llvmPtrTy = mlir::LLVM::LLVMPointerType::get(rewriter.getContext());
   mlir::Location loc = op.getLoc();
 
   // %landingpad = landingpad { ptr, i32 }
   // Note that since llvm.landingpad has to be the first operation on the
   // block, any needed value for its operands has to be added somewhere else.
-  if (symListAttr) {
+  if (catchListAttr) {
     //   catch ptr @_ZTIi
     //   catch ptr @_ZTIPKc
-    for (mlir::Attribute attr : symListAttr) {
-      auto symAttr = cast<mlir::FlatSymbolRefAttr>(attr);
+    for (mlir::Attribute catchAttr : catchListAttr) {
+      auto symAttr = cast<mlir::FlatSymbolRefAttr>(catchAttr);
       // Generate `llvm.mlir.addressof` for each symbol, and place those
       // operations in the LLVM function entry basic block.
       mlir::OpBuilder::InsertionGuard guard(rewriter);
       rewriter.setInsertionPointToStart(entryBlock);
       mlir::Value addrOp = mlir::LLVM::AddressOfOp::create(
           rewriter, loc, llvmPtrTy, symAttr.getValue());
-      symAddrs.push_back(addrOp);
+      catchSymAddrs.push_back(addrOp);
     }
   } else if (!op.getCleanup()) {
-    // catch ptr null
+    // We need to emit catch-all only if cleanup is not set, because when we
+    // have catch-all handler, there is no case when we set would unwind past
+    // the handler
     mlir::OpBuilder::InsertionGuard guard(rewriter);
     rewriter.setInsertionPointToStart(entryBlock);
     mlir::Value nullOp = mlir::LLVM::ZeroOp::create(rewriter, loc, llvmPtrTy);
-    symAddrs.push_back(nullOp);
+    catchSymAddrs.push_back(nullOp);
   }
 
   // %slot = extractvalue { ptr, i32 } %x, 0
   // %selector = extractvalue { ptr, i32 } %x, 1
   mlir::LLVM::LLVMStructType llvmLandingPadStructTy =
       getLLVMLandingPadStructTy(rewriter);
   auto landingPadOp = mlir::LLVM::LandingpadOp::create(
-      rewriter, loc, llvmLandingPadStructTy, symAddrs);
+      rewriter, loc, llvmLandingPadStructTy, catchSymAddrs);
 
   if (op.getCleanup())
     landingPadOp.setCleanup(true);
@@ -3139,7 +3141,7 @@ mlir::LogicalResult CIRToLLVMEhInflightOpLowering::matchAndRewrite(
   auto personalityFnTy =
       mlir::LLVM::LLVMFunctionType::get(rewriter.getI32Type(), {},
                                         /*isVarArg=*/true);
-  // Get or create `__gxx_personality_v0`
+
   const StringRef fnName = "__gxx_personality_v0";
   createLLVMFuncOpIfNotExist(rewriter, op, fnName, personalityFnTy);
   llvmFn.setPersonality(fnName);
diff --git a/clang/test/CIR/IR/eh-inflight.cir b/clang/test/CIR/IR/eh-inflight.cir
@@ -1,15 +1,40 @@
 // RUN: cir-opt %s --verify-roundtrip | FileCheck %s
 
+!u8i = !cir.int<u, 8>
+
 module {
 
-cir.func dso_local @function_with_inflight_exception() {
+cir.func dso_local @inflight_exception() {
   %exception_ptr, %type_id = cir.eh.inflight_exception
   cir.return
 }
 
-// CHECK: cir.func dso_local @function_with_inflight_exception() {
+// CHECK: cir.func dso_local @inflight_exception() {
 // CHECK:  %exception_ptr, %type_id = cir.eh.inflight_exception
 // CHECK:  cir.return
 // CHECK: }
 
+cir.func dso_local @inflight_exception_with_cleanup() {
+  %exception_ptr, %type_id = cir.eh.inflight_exception cleanup
+  cir.return
+}
+
+// CHECK: cir.func dso_local @inflight_exception_with_cleanup() {
+// CHECK:  %exception_ptr, %type_id = cir.eh.inflight_exception cleanup
+// CHECK:  cir.return
+// CHECK: }
+
+cir.global "private" constant external @_ZTIi : !cir.ptr<!u8i>
+cir.global "private" constant external @_ZTIPKc : !cir.ptr<!u8i>
+
+cir.func dso_local @inflight_exception_with_catch_type_list() {
+  %exception_ptr, %type_id = cir.eh.inflight_exception [@_ZTIi, @_ZTIPKc]
+  cir.return
+}
+
+// CHECK: cir.func dso_local @inflight_exception_with_catch_type_list() {
+// CHECK:  %exception_ptr, %type_id = cir.eh.inflight_exception [@_ZTIi, @_ZTIPKc]
+// CHECK:  cir.return
+// CHECK:}
+
 }
diff --git a/clang/test/CIR/Lowering/eh-inflight.cir b/clang/test/CIR/Lowering/eh-inflight.cir
@@ -0,0 +1,53 @@
+// RUN: cir-opt %s -cir-to-llvm -o %t.cir
+
+!u8i = !cir.int<u, 8>
+
+module {
+
+// CHECK: llvm.func @__gxx_personality_v0(...) -> i32
+
+cir.func @inflight_exception() {
+  %exception_ptr, %type_id = cir.eh.inflight_exception
+  cir.return
+}
+
+// CHECK: llvm.func @inflight_exception() attributes {personality = @__gxx_personality_v0} {
+// CHECK:   %[[CONST_0:.*]] = llvm.mlir.zero : !llvm.ptr
+// CHECK:   %[[LP:.*]] = llvm.landingpad (catch %[[CONST_0]] : !llvm.ptr) : !llvm.struct<(ptr, i32)>
+// CHECK:   %[[EXCEPTION_PTR:.*]] = llvm.extractvalue %[[LP]][0] : !llvm.struct<(ptr, i32)> 
+// CHECK:   %[[TYPE_ID:.*]] = llvm.extractvalue %[[LP]][1] : !llvm.struct<(ptr, i32)> 
+// CHECK:   llvm.return
+// CHECK: }
+
+cir.func @inflight_exception_with_cleanup() {
+  %exception_ptr, %type_id = cir.eh.inflight_exception cleanup
+  cir.return
+}
+
+// CHECK: llvm.func @inflight_exception_with_cleanup() attributes {personality = @__gxx_personality_v0} {
+// CHECK:   %[[LP:.*]] = llvm.landingpad cleanup : !llvm.struct<(ptr, i32)>
+// CHECK:   %[[EXCEPTION_PTR:.*]] = llvm.extractvalue %[[LP]][0] : !llvm.struct<(ptr, i32)> 
+// CHECK:   %[[TYPE_ID:.*]] = llvm.extractvalue %[[LP]][1] : !llvm.struct<(ptr, i32)> 
+// CHECK:   llvm.return
+// CHECK: }
+
+
+cir.global "private" constant external @_ZTIi : !cir.ptr<!u8i>
+cir.global "private" constant external @_ZTIPKc : !cir.ptr<!u8i>
+
+cir.func @inflight_exception_with_catch_type_list() {
+  %exception_ptr, %type_id = cir.eh.inflight_exception [@_ZTIi, @_ZTIPKc]
+  cir.return
+}
+
+// CHECK: llvm.func @inflight_exception_with_catch_type_list() attributes {personality = @__gxx_personality_v0} {
+// CHECK:   %[[TI_1_ADDR:.*]] = llvm.mlir.addressof @_ZTIPKc : !llvm.ptr
+// CHECK:   %[[TI_2_ADDR:.*]] = llvm.mlir.addressof @_ZTIi : !llvm.ptr
+// CHECK:   %[[LP:.*]] = llvm.landingpad (catch %[[TI_2_ADDR]] : !llvm.ptr) (catch %[[TI_1_ADDR]] : !llvm.ptr) : !llvm.struct<(ptr, i32)>
+// CHECK:   %[[EXCEPTION_PTR:.*]] = llvm.extractvalue %[[LP]][0] : !llvm.struct<(ptr, i32)> 
+// CHECK:   %[[TYPE_ID:.*]] = llvm.extractvalue %[[LP]][1] : !llvm.struct<(ptr, i32)> 
+// CHECK:   llvm.return
+// CHECK: }
+
+
+}
