[CIR] Implement function personality attribute and its lowering

[xlauko]

No description provided.

[xlauko]

• [CIR] Make CIR-to-LLVM a one shot conversion #171083
• [CIR] Implement function personality attribute and its lowering #171001 👈 (View in Graphite)
• [CIR] Clean up visibility conversion (NFC) #171000
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-clangir

@llvm/pr-subscribers-clang

Author: Henrich Lauko (xlauko)

Changes

---

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

8 Files Affected:

• (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+5)
• (modified) clang/include/clang/CIR/MissingFeatures.h (+1-1)
• (modified) clang/lib/CIR/CodeGen/CIRGenException.cpp (+18-2)
• (modified) clang/lib/CIR/CodeGen/CIRGenModule.cpp (+17-4)
• (modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+18)
• (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+4-16)
• (modified) clang/test/CIR/IR/func.cir (+8)
• (modified) clang/test/CIR/Lowering/eh-inflight.cir (+6-3)

diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index caa047a51b689..3d6de2a97d650 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -2676,6 +2676,10 @@ def CIR_FuncOp : CIR_Op<"func", [
     The `always_inline` attribute marks a function that should always be inlined.
     The `inline_hint` attribute suggests that the function should be inlined.
 
+    The `personality` attribute specifies the personality function to use for
+    exception handling. This is a symbol reference to the personality function
+    (e.g., `@__gxx_personality_v0` for C++ exceptions).
+
     Example:
 
     ```mlir
@@ -2722,6 +2726,7 @@ def CIR_FuncOp : CIR_Op<"func", [
     OptionalAttr<DictArrayAttr>:$arg_attrs,
     OptionalAttr<DictArrayAttr>:$res_attrs,
     OptionalAttr<FlatSymbolRefAttr>:$aliasee,
+    OptionalAttr<FlatSymbolRefAttr>:$personality,
     CIR_OptionalPriorityAttr:$global_ctor_priority,
     CIR_OptionalPriorityAttr:$global_dtor_priority,
     OptionalAttr<CIR_CXXSpecialMemberAttr>:$cxx_special_member
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index 826a4b13f5c0c..95b3bfa58956e 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -93,7 +93,6 @@ struct MissingFeatures {
   static bool opFuncNoReturn() { return false; }
   static bool setFunctionAttributes() { return false; }
   static bool setLLVMFunctionFEnvAttributes() { return false; }
-  static bool setFunctionPersonality() { return false; }
 
   // CallOp handling
   static bool opCallAggregateArgs() { return false; }
@@ -274,6 +273,7 @@ struct MissingFeatures {
 
   static bool fpConstraints() { return false; }
   static bool generateDebugInfo() { return false; }
+  static bool getRuntimeFunctionDecl() { return false; }
   static bool globalViewIndices() { return false; }
   static bool globalViewIntLowering() { return false; }
   static bool handleBuiltinICEArguments() { return false; }
diff --git a/clang/lib/CIR/CodeGen/CIRGenException.cpp b/clang/lib/CIR/CodeGen/CIRGenException.cpp
index 375828421eb1b..53165bf3d1b83 100644
--- a/clang/lib/CIR/CodeGen/CIRGenException.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenException.cpp
@@ -185,6 +185,18 @@ const EHPersonality &EHPersonality::get(CIRGenFunction &cgf) {
   return get(cgf.cgm, dyn_cast_or_null<FunctionDecl>(fg));
 }
 
+static llvm::StringRef 
+getPersonalityFn(CIRGenModule &cgm, const EHPersonality &personality) {
+  // Create the personality function type: i32 (...)
+  mlir::Type i32Ty = cgm.getBuilder().getI32Type();
+  auto funcTy = cir::FuncType::get({}, i32Ty, /*isVarArg=*/true);
+  
+  cir::FuncOp personalityFn = cgm.createRuntimeFunction(
+      funcTy, personality.personalityFn, mlir::ArrayAttr(), /*isLocal=*/true);
+  
+  return personalityFn.getSymName();
+}
+
 void CIRGenFunction::emitCXXThrowExpr(const CXXThrowExpr *e) {
   const llvm::Triple &triple = getTarget().getTriple();
   if (cgm.getLangOpts().OpenMPIsTargetDevice &&
@@ -640,10 +652,14 @@ void CIRGenFunction::populateCatchHandlersIfRequired(cir::TryOp tryOp) {
   assert(ehStack.requiresCatchOrCleanup());
   assert(!ehStack.empty());
 
-  assert(!cir::MissingFeatures::setFunctionPersonality());
+  const EHPersonality &personality = EHPersonality::get(*this);
+  
+  // Set personality function if not already set
+  auto funcOp = mlir::cast<cir::FuncOp>(curFn);
+  if (!funcOp.getPersonality())
+    funcOp.setPersonality(getPersonalityFn(cgm, personality));
 
   // CIR does not cache landing pads.
-  const EHPersonality &personality = EHPersonality::get(*this);
   if (personality.usesFuncletPads()) {
     cgm.errorNYI("getInvokeDestImpl: usesFuncletPads");
   } else {
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
index e1894c040dd53..f784eb929248e 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
@@ -2301,14 +2301,27 @@ void CIRGenModule::setCXXSpecialMemberAttr(
   }
 }
 
+static void setWindowsItaniumDLLImport(CIRGenModule &cgm, bool isLocal,
+                                       cir::FuncOp funcOp, StringRef name) {
+  // In Windows Itanium environments, try to mark runtime functions
+  // dllimport. For Mingw and MSVC, don't. We don't really know if the user
+  // will link their standard library statically or dynamically. Marking
+  // functions imported when they are not imported can cause linker errors
+  // and warnings.
+  if (!isLocal && cgm.getTarget().getTriple().isWindowsItaniumEnvironment() &&
+      !cgm.getCodeGenOpts().LTOVisibilityPublicStd) {
+    assert(!cir::MissingFeatures::getRuntimeFunctionDecl());
+    assert(!cir::MissingFeatures::setDLLStorageClass());
+    assert(!cir::MissingFeatures::opGlobalDLLImportExport());
+  }
+}
+
 cir::FuncOp CIRGenModule::createRuntimeFunction(cir::FuncType ty,
                                                 StringRef name, mlir::ArrayAttr,
-                                                [[maybe_unused]] bool isLocal,
+                                                bool isLocal,
                                                 bool assumeConvergent) {
   if (assumeConvergent)
     errorNYI("createRuntimeFunction: assumeConvergent");
-  if (isLocal)
-    errorNYI("createRuntimeFunction: local");
 
   cir::FuncOp entry = getOrCreateCIRFunction(name, ty, GlobalDecl(),
                                              /*forVtable=*/false);
@@ -2317,7 +2330,7 @@ cir::FuncOp CIRGenModule::createRuntimeFunction(cir::FuncType ty,
     // TODO(cir): set the attributes of the function.
     assert(!cir::MissingFeatures::setLLVMFunctionFEnvAttributes());
     assert(!cir::MissingFeatures::opFuncCallingConv());
-    assert(!cir::MissingFeatures::opGlobalDLLImportExport());
+    setWindowsItaniumDLLImport(*this, isLocal, entry, name);
     entry.setDSOLocal(true);
   }
 
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index ec8cae62d6bc8..c5f19ab584fb8 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -1939,6 +1939,18 @@ ParseResult cir::FuncOp::parse(OpAsmParser &parser, OperationState &state) {
     hasAlias = true;
   }
 
+  mlir::StringAttr personalityNameAttr = getPersonalityAttrName(state.name);
+  if (parser.parseOptionalKeyword("personality").succeeded()) {
+    if (parser.parseLParen().failed())
+      return failure();
+    mlir::StringAttr personalityAttr;
+    if (parser.parseOptionalSymbolName(personalityAttr).failed())
+      return failure();
+    state.addAttribute(personalityNameAttr, FlatSymbolRefAttr::get(personalityAttr));
+    if (parser.parseRParen().failed())
+      return failure();
+  }
+
   auto parseGlobalDtorCtor =
       [&](StringRef keyword,
           llvm::function_ref<void(std::optional<int> prio)> createAttr)
@@ -2140,6 +2152,12 @@ void cir::FuncOp::print(OpAsmPrinter &p) {
     p << ")";
   }
 
+  if (std::optional<StringRef> personalityName = getPersonality()) {
+    p << " personality(";
+    p.printSymbolName(*personalityName);
+    p << ")";
+  }
+
   if (auto specialMemberAttr = getCxxSpecialMember()) {
     p << " special_member<";
     p.printAttribute(*specialMemberAttr);
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index cf1a7dcd77ecd..8489616b88cef 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -2015,6 +2015,10 @@ mlir::LogicalResult CIRToLLVMFuncOpLowering::matchAndRewrite(
     fn.setAlwaysInline(*inlineKind == cir::InlineKind::AlwaysInline);
   }
 
+  // Transfer personality attribute from CIR to LLVM if present
+  if (std::optional<llvm::StringRef> personality = op.getPersonality())
+    fn.setPersonality(*personality);
+
   fn.setVisibility_(lowerCIRVisibilityToLLVMVisibility(
                         op.getGlobalVisibility()));
 
@@ -3342,22 +3346,6 @@ mlir::LogicalResult CIRToLLVMEhInflightOpLowering::matchAndRewrite(
       mlir::LLVM::ExtractValueOp::create(rewriter, loc, landingPadOp, 1);
   rewriter.replaceOp(op, mlir::ValueRange{slot, selector});
 
-  // Landing pads are required to be in LLVM functions with personality
-  // attribute.
-  // TODO(cir): for now hardcode personality creation in order to start
-  // adding exception tests, once we annotate CIR with such information,
-  // change it to be in FuncOp lowering instead.
-  mlir::OpBuilder::InsertionGuard guard(rewriter);
-  // Insert personality decl before the current function.
-  rewriter.setInsertionPoint(llvmFn);
-  auto personalityFnTy =
-      mlir::LLVM::LLVMFunctionType::get(rewriter.getI32Type(), {},
-                                        /*isVarArg=*/true);
-
-  const StringRef fnName = "__gxx_personality_v0";
-  createLLVMFuncOpIfNotExist(rewriter, op, fnName, personalityFnTy);
-  llvmFn.setPersonality(fnName);
-
   return mlir::success();
 }
 
diff --git a/clang/test/CIR/IR/func.cir b/clang/test/CIR/IR/func.cir
index d8906ab3e1301..52589c8a5e39e 100644
--- a/clang/test/CIR/IR/func.cir
+++ b/clang/test/CIR/IR/func.cir
@@ -44,6 +44,14 @@ cir.func @intfunc() -> !s32i {
 cir.func @a_empty() alias(@empty)
 // CHECK: cir.func @a_empty() alias(@empty)
 
+// Should print/parse function personality.
+cir.func @personality_func() personality(@__gxx_personality_v0) {
+  cir.return
+}
+// CHECK: cir.func @personality_func() personality(@__gxx_personality_v0) {
+// CHECK:   cir.return
+// CHECK: }
+
 // int scopes() {
 //  {
 //    {
diff --git a/clang/test/CIR/Lowering/eh-inflight.cir b/clang/test/CIR/Lowering/eh-inflight.cir
index 31e1e474a046b..89f585f439fa4 100644
--- a/clang/test/CIR/Lowering/eh-inflight.cir
+++ b/clang/test/CIR/Lowering/eh-inflight.cir
@@ -1,12 +1,15 @@
 // RUN: cir-opt %s -cir-to-llvm -o %t.cir
 
+!s32i = !cir.int<s, 32>
 !u8i = !cir.int<u, 8>
 
 module {
 
+cir.func private @__gxx_personality_v0(...) -> !s32i
+
 // CHECK: llvm.func @__gxx_personality_v0(...) -> i32
 
-cir.func @inflight_exception() {
+cir.func @inflight_exception() personality(@__gxx_personality_v0) {
   %exception_ptr, %type_id = cir.eh.inflight_exception
   cir.return
 }
@@ -19,7 +22,7 @@ cir.func @inflight_exception() {
 // CHECK:   llvm.return
 // CHECK: }
 
-cir.func @inflight_exception_with_cleanup() {
+cir.func @inflight_exception_with_cleanup() personality(@__gxx_personality_v0) {
   %exception_ptr, %type_id = cir.eh.inflight_exception cleanup
   cir.return
 }
@@ -35,7 +38,7 @@ cir.func @inflight_exception_with_cleanup() {
 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() {
+cir.func @inflight_exception_with_catch_type_list() personality(@__gxx_personality_v0) {
   %exception_ptr, %type_id = cir.eh.inflight_exception [@_ZTIi, @_ZTIPKc]
   cir.return
 }

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[AmrDeveloper]

LGTM, Thanks

[xlauko]

Merge activity

• Dec 8, 3:14 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Dec 8, 4:41 PM UTC: Graphite couldn't merge this pull request because a downstack PR [CIR] Clean up visibility conversion (NFC) #171000 failed to merge.

[andykaylor]

This looks good except that the CIRGenModule and CIRGenException changes are not tested.

[andykaylor]

Is it possible to test this or does it require additional implementation?

[xlauko]

This looks good except that the CIRGenModule and CIRGenException changes are not tested.

What do you mean by CIRGenModule changes? I just added missing feature test, as setWindowsItaniumDLLImport is unconditionally invoked on the path for personality function generation, but it is still not supported.

I believe CIRGenException changes were already tested, I just moved personality funciton generation from LowerToLLVM.

[andykaylor]

What do you mean by CIRGenModule changes? I just added missing feature test, as setWindowsItaniumDLLImport is unconditionally invoked on the path for personality function generation, but it is still not supported.

Yeah, sorry for the confusion. I didn't notice that was the only change in CIRGenModule.

I believe CIRGenException changes were already tested, I just moved personality funciton generation from LowerToLLVM.

The clang/test/CIR/Lowering/eh-inflight.cir test only covers the lowering from CIR case. There is no test that handles the generation of the personality attribute from C++ source.

[xlauko]

What do you mean by CIRGenModule changes? I just added missing feature test, as setWindowsItaniumDLLImport is unconditionally invoked on the path for personality function generation, but it is still not supported.

Yeah, sorry for the confusion. I didn't notice that was the only change in CIRGenModule.

I believe CIRGenException changes were already tested, I just moved personality funciton generation from LowerToLLVM.

The clang/test/CIR/Lowering/eh-inflight.cir test only covers the lowering from CIR case. There is no test that handles the generation of the personality attribute from C++ source.

True, I will add it. FYI I am fixing this as I am trying to make LowerToLLVM to be a one-shot conversion and the adhoc personality conversion showed up as an issue.