Rollup merge of #132342 - Zalathar:operand-bundle, r=workingjubilee

cg_llvm: Clean up FFI calls for operand bundles

All of these FFI functions have equivalents in the stable LLVM-C API, though `LLVMBuildCallBr` requires a temporary polyfill on LLVM 18.

This PR also creates a clear split between `OperandBundleOwned` and `OperandBundle`, and updates the internals of the owner to be a little less terrifying.

compiler/rustc_codegen_llvm/src/allocator.rs

@@ -154,7 +154,7 @@ fn create_wrapper_function(
             .enumerate()
             .map(|(i, _)| llvm::LLVMGetParam(llfn, i as c_uint))
             .collect::<Vec<_>>();
-        let ret = llvm::LLVMRustBuildCall(
+        let ret = llvm::LLVMBuildCallWithOperandBundles(
             llbuilder,
             ty,
             callee,
@@ -162,6 +162,7 @@ fn create_wrapper_function(
             args.len() as c_uint,
             [].as_ptr(),
             0 as c_uint,
+            c"".as_ptr(),
         );
         llvm::LLVMSetTailCall(ret, True);
         if output.is_some() {

compiler/rustc_codegen_llvm/src/builder.rs

@@ -239,7 +239,6 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
 
         let args = self.check_call("invoke", llty, llfn, args);
         let funclet_bundle = funclet.map(|funclet| funclet.bundle());
-        let funclet_bundle = funclet_bundle.as_ref().map(|b| &*b.raw);
         let mut bundles: SmallVec<[_; 2]> = SmallVec::new();
         if let Some(funclet_bundle) = funclet_bundle {
             bundles.push(funclet_bundle);
@@ -250,13 +249,12 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
 
         // Emit KCFI operand bundle
         let kcfi_bundle = self.kcfi_operand_bundle(fn_attrs, fn_abi, instance, llfn);
-        let kcfi_bundle = kcfi_bundle.as_ref().map(|b| &*b.raw);
+        if let Some(kcfi_bundle) = kcfi_bundle.as_deref() {
-        if let Some(kcfi_bundle) = kcfi_bundle {
             bundles.push(kcfi_bundle);
         }
 
         let invoke = unsafe {
-            llvm::LLVMRustBuildInvoke(
+            llvm::LLVMBuildInvokeWithOperandBundles(
                 self.llbuilder,
                 llty,
                 llfn,
@@ -1179,7 +1177,6 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
 
         let args = self.check_call("call", llty, llfn, args);
         let funclet_bundle = funclet.map(|funclet| funclet.bundle());
-        let funclet_bundle = funclet_bundle.as_ref().map(|b| &*b.raw);
         let mut bundles: SmallVec<[_; 2]> = SmallVec::new();
         if let Some(funclet_bundle) = funclet_bundle {
             bundles.push(funclet_bundle);
@@ -1190,13 +1187,12 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
 
         // Emit KCFI operand bundle
         let kcfi_bundle = self.kcfi_operand_bundle(fn_attrs, fn_abi, instance, llfn);
-        let kcfi_bundle = kcfi_bundle.as_ref().map(|b| &*b.raw);
+        if let Some(kcfi_bundle) = kcfi_bundle.as_deref() {
-        if let Some(kcfi_bundle) = kcfi_bundle {
             bundles.push(kcfi_bundle);
         }
 
         let call = unsafe {
-            llvm::LLVMRustBuildCall(
+            llvm::LLVMBuildCallWithOperandBundles(
                 self.llbuilder,
                 llty,
                 llfn,
@@ -1204,6 +1200,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
                 args.len() as c_uint,
                 bundles.as_ptr(),
                 bundles.len() as c_uint,
+                c"".as_ptr(),
             )
         };
         if let Some(fn_abi) = fn_abi {
@@ -1509,7 +1506,6 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
 
         let args = self.check_call("callbr", llty, llfn, args);
         let funclet_bundle = funclet.map(|funclet| funclet.bundle());
-        let funclet_bundle = funclet_bundle.as_ref().map(|b| &*b.raw);
         let mut bundles: SmallVec<[_; 2]> = SmallVec::new();
         if let Some(funclet_bundle) = funclet_bundle {
             bundles.push(funclet_bundle);
@@ -1520,13 +1516,12 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
 
         // Emit KCFI operand bundle
         let kcfi_bundle = self.kcfi_operand_bundle(fn_attrs, fn_abi, instance, llfn);
-        let kcfi_bundle = kcfi_bundle.as_ref().map(|b| &*b.raw);
+        if let Some(kcfi_bundle) = kcfi_bundle.as_deref() {
-        if let Some(kcfi_bundle) = kcfi_bundle {
             bundles.push(kcfi_bundle);
         }
 
         let callbr = unsafe {
-            llvm::LLVMRustBuildCallBr(
+            llvm::LLVMBuildCallBr(
                 self.llbuilder,
                 llty,
                 llfn,
@@ -1601,7 +1596,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
         fn_abi: Option<&FnAbi<'tcx, Ty<'tcx>>>,
         instance: Option<Instance<'tcx>>,
         llfn: &'ll Value,
-    ) -> Option<llvm::OperandBundleDef<'ll>> {
+    ) -> Option<llvm::OperandBundleOwned<'ll>> {
         let is_indirect_call = unsafe { llvm::LLVMRustIsNonGVFunctionPointerTy(llfn) };
         let kcfi_bundle = if self.tcx.sess.is_sanitizer_kcfi_enabled()
             && let Some(fn_abi) = fn_abi
@@ -1627,7 +1622,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
                 kcfi::typeid_for_fnabi(self.tcx, fn_abi, options)
             };
 
-            Some(llvm::OperandBundleDef::new("kcfi", &[self.const_u32(kcfi_typeid)]))
+            Some(llvm::OperandBundleOwned::new("kcfi", &[self.const_u32(kcfi_typeid)]))
         } else {
             None
         };

compiler/rustc_codegen_llvm/src/common.rs

@@ -17,7 +17,7 @@ use tracing::debug;
 
 use crate::consts::const_alloc_to_llvm;
 pub(crate) use crate::context::CodegenCx;
-use crate::llvm::{self, BasicBlock, Bool, ConstantInt, False, Metadata, OperandBundleDef, True};
+use crate::llvm::{self, BasicBlock, Bool, ConstantInt, False, Metadata, True};
 use crate::type_::Type;
 use crate::value::Value;
 
@@ -63,19 +63,19 @@ use crate::value::Value;
 /// the `OperandBundleDef` value created for MSVC landing pads.
 pub(crate) struct Funclet<'ll> {
     cleanuppad: &'ll Value,
-    operand: OperandBundleDef<'ll>,
+    operand: llvm::OperandBundleOwned<'ll>,
 }
 
 impl<'ll> Funclet<'ll> {
     pub(crate) fn new(cleanuppad: &'ll Value) -> Self {
-        Funclet { cleanuppad, operand: OperandBundleDef::new("funclet", &[cleanuppad]) }
+        Funclet { cleanuppad, operand: llvm::OperandBundleOwned::new("funclet", &[cleanuppad]) }
     }
 
     pub(crate) fn cleanuppad(&self) -> &'ll Value {
         self.cleanuppad
     }
 
-    pub(crate) fn bundle(&self) -> &OperandBundleDef<'ll> {
+    pub(crate) fn bundle(&self) -> &llvm::OperandBundle<'ll> {
         &self.operand
     }
 }

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

@@ -3,6 +3,7 @@
 
 use std::fmt::Debug;
 use std::marker::PhantomData;
+use std::ptr;
 
 use libc::{c_char, c_int, c_uint, c_ulonglong, c_void, size_t};
 use rustc_macros::TryFromU32;
@@ -708,8 +709,9 @@ unsafe extern "C" {
 }
 #[repr(C)]
 pub struct RustArchiveMember<'a>(InvariantOpaque<'a>);
+/// Opaque pointee of `LLVMOperandBundleRef`.
 #[repr(C)]
-pub struct OperandBundleDef<'a>(InvariantOpaque<'a>);
+pub(crate) struct OperandBundle<'a>(InvariantOpaque<'a>);
 #[repr(C)]
 pub struct Linker<'a>(InvariantOpaque<'a>);
 
@@ -1538,6 +1540,50 @@ unsafe extern "C" {
 
     pub fn LLVMGetOrInsertComdat(M: &Module, Name: *const c_char) -> &Comdat;
     pub fn LLVMSetComdat(V: &Value, C: &Comdat);
+
+    pub(crate) fn LLVMCreateOperandBundle(
+        Tag: *const c_char,
+        TagLen: size_t,
+        Args: *const &'_ Value,
+        NumArgs: c_uint,
+    ) -> *mut OperandBundle<'_>;
+    pub(crate) fn LLVMDisposeOperandBundle(Bundle: ptr::NonNull<OperandBundle<'_>>);
+
+    pub(crate) fn LLVMBuildCallWithOperandBundles<'a>(
+        B: &Builder<'a>,
+        Ty: &'a Type,
+        Fn: &'a Value,
+        Args: *const &'a Value,
+        NumArgs: c_uint,
+        Bundles: *const &OperandBundle<'a>,
+        NumBundles: c_uint,
+        Name: *const c_char,
+    ) -> &'a Value;
+    pub(crate) fn LLVMBuildInvokeWithOperandBundles<'a>(
+        B: &Builder<'a>,
+        Ty: &'a Type,
+        Fn: &'a Value,
+        Args: *const &'a Value,
+        NumArgs: c_uint,
+        Then: &'a BasicBlock,
+        Catch: &'a BasicBlock,
+        Bundles: *const &OperandBundle<'a>,
+        NumBundles: c_uint,
+        Name: *const c_char,
+    ) -> &'a Value;
+    pub(crate) fn LLVMBuildCallBr<'a>(
+        B: &Builder<'a>,
+        Ty: &'a Type,
+        Fn: &'a Value,
+        DefaultDest: &'a BasicBlock,
+        IndirectDests: *const &'a BasicBlock,
+        NumIndirectDests: c_uint,
+        Args: *const &'a Value,
+        NumArgs: c_uint,
+        Bundles: *const &OperandBundle<'a>,
+        NumBundles: c_uint,
+        Name: *const c_char,
+    ) -> &'a Value;
 }
 
 #[link(name = "llvm-wrapper", kind = "static")]
@@ -1623,47 +1669,11 @@ unsafe extern "C" {
         AttrsLen: size_t,
     );
 
-    pub fn LLVMRustBuildInvoke<'a>(
-        B: &Builder<'a>,
-        Ty: &'a Type,
-        Fn: &'a Value,
-        Args: *const &'a Value,
-        NumArgs: c_uint,
-        Then: &'a BasicBlock,
-        Catch: &'a BasicBlock,
-        OpBundles: *const &OperandBundleDef<'a>,
-        NumOpBundles: c_uint,
-        Name: *const c_char,
-    ) -> &'a Value;
-
-    pub fn LLVMRustBuildCallBr<'a>(
-        B: &Builder<'a>,
-        Ty: &'a Type,
-        Fn: &'a Value,
-        DefaultDest: &'a BasicBlock,
-        IndirectDests: *const &'a BasicBlock,
-        NumIndirectDests: c_uint,
-        Args: *const &'a Value,
-        NumArgs: c_uint,
-        OpBundles: *const &OperandBundleDef<'a>,
-        NumOpBundles: c_uint,
-        Name: *const c_char,
-    ) -> &'a Value;
-
     pub fn LLVMRustSetFastMath(Instr: &Value);
     pub fn LLVMRustSetAlgebraicMath(Instr: &Value);
     pub fn LLVMRustSetAllowReassoc(Instr: &Value);
 
     // Miscellaneous instructions
-    pub fn LLVMRustBuildCall<'a>(
-        B: &Builder<'a>,
-        Ty: &'a Type,
-        Fn: &'a Value,
-        Args: *const &'a Value,
-        NumArgs: c_uint,
-        OpBundles: *const &OperandBundleDef<'a>,
-        NumOpBundles: c_uint,
-    ) -> &'a Value;
     pub fn LLVMRustBuildMemCpy<'a>(
         B: &Builder<'a>,
         Dst: &'a Value,
@@ -2357,13 +2367,6 @@ unsafe extern "C" {
 
     pub fn LLVMRustSetDataLayoutFromTargetMachine<'a>(M: &'a Module, TM: &'a TargetMachine);
 
-    pub fn LLVMRustBuildOperandBundleDef(
-        Name: *const c_char,
-        Inputs: *const &'_ Value,
-        NumInputs: c_uint,
-    ) -> &mut OperandBundleDef<'_>;
-    pub fn LLVMRustFreeOperandBundleDef<'a>(Bundle: &'a mut OperandBundleDef<'a>);
-
     pub fn LLVMRustPositionBuilderAtStart<'a>(B: &Builder<'a>, BB: &'a BasicBlock);
 
     pub fn LLVMRustSetModulePICLevel(M: &Module);

compiler/rustc_codegen_llvm/src/llvm/mod.rs

@@ -2,11 +2,12 @@
 
 use std::cell::RefCell;
 use std::ffi::{CStr, CString};
+use std::ops::Deref;
+use std::ptr;
 use std::str::FromStr;
 use std::string::FromUtf8Error;
 
 use libc::c_uint;
-use rustc_data_structures::small_c_str::SmallCStr;
 use rustc_llvm::RustString;
 use rustc_target::abi::{Align, Size, WrappingRange};
 
@@ -331,28 +332,43 @@ pub fn last_error() -> Option<String> {
     }
 }
 
-pub struct OperandBundleDef<'a> {
+/// Owns an [`OperandBundle`], and will dispose of it when dropped.
-    pub raw: &'a mut ffi::OperandBundleDef<'a>,
+pub(crate) struct OperandBundleOwned<'a> {
+    raw: ptr::NonNull<OperandBundle<'a>>,
 }
 
-impl<'a> OperandBundleDef<'a> {
+impl<'a> OperandBundleOwned<'a> {
-    pub fn new(name: &str, vals: &[&'a Value]) -> Self {
+    pub(crate) fn new(name: &str, vals: &[&'a Value]) -> Self {
-        let name = SmallCStr::new(name);
+        let raw = unsafe {
-        let def = unsafe {
+            LLVMCreateOperandBundle(
-            LLVMRustBuildOperandBundleDef(name.as_ptr(), vals.as_ptr(), vals.len() as c_uint)
+                name.as_c_char_ptr(),
+                name.len(),
+                vals.as_ptr(),
+                vals.len() as c_uint,
+            )
         };
-        OperandBundleDef { raw: def }
+        OperandBundleOwned { raw: ptr::NonNull::new(raw).unwrap() }
     }
 }
 
-impl Drop for OperandBundleDef<'_> {
+impl Drop for OperandBundleOwned<'_> {
     fn drop(&mut self) {
         unsafe {
-            LLVMRustFreeOperandBundleDef(&mut *(self.raw as *mut _));
+            LLVMDisposeOperandBundle(self.raw);
         }
     }
 }
 
+impl<'a> Deref for OperandBundleOwned<'a> {
+    type Target = OperandBundle<'a>;
+
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: The returned reference is opaque and can only used for FFI.
+        // It is valid for as long as `&self` is.
+        unsafe { self.raw.as_ref() }
+    }
+}
+
 pub(crate) fn add_module_flag_u32(
     module: &Module,
     merge_behavior: ModuleFlagMergeBehavior,

compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

@@ -1537,38 +1537,6 @@ LLVMRustUnpackSMDiagnostic(LLVMSMDiagnosticRef DRef, RustStringRef MessageOut,
   return true;
 }
 
-extern "C" OperandBundleDef *LLVMRustBuildOperandBundleDef(const char *Name,
-                                                           LLVMValueRef *Inputs,
-                                                           unsigned NumInputs) {
-  return new OperandBundleDef(Name,
-                              ArrayRef<Value *>(unwrap(Inputs), NumInputs));
-}
-
-extern "C" void LLVMRustFreeOperandBundleDef(OperandBundleDef *Bundle) {
-  delete Bundle;
-}
-
-// OpBundlesIndirect is an array of pointers (*not* a pointer to an array).
-extern "C" LLVMValueRef LLVMRustBuildCall(LLVMBuilderRef B, LLVMTypeRef Ty,
-                                          LLVMValueRef Fn, LLVMValueRef *Args,
-                                          unsigned NumArgs,
-                                          OperandBundleDef **OpBundlesIndirect,
-                                          unsigned NumOpBundles) {
-  Value *Callee = unwrap(Fn);
-  FunctionType *FTy = unwrap<FunctionType>(Ty);
-
-  // FIXME: Is there a way around this?
-  SmallVector<OperandBundleDef> OpBundles;
-  OpBundles.reserve(NumOpBundles);
-  for (unsigned i = 0; i < NumOpBundles; ++i) {
-    OpBundles.push_back(*OpBundlesIndirect[i]);
-  }
-
-  return wrap(unwrap(B)->CreateCall(FTy, Callee,
-                                    ArrayRef<Value *>(unwrap(Args), NumArgs),
-                                    ArrayRef<OperandBundleDef>(OpBundles)));
-}
-
 extern "C" LLVMValueRef LLVMRustBuildMemCpy(LLVMBuilderRef B, LLVMValueRef Dst,
                                             unsigned DstAlign, LLVMValueRef Src,
                                             unsigned SrcAlign,
@@ -1596,37 +1564,18 @@ extern "C" LLVMValueRef LLVMRustBuildMemSet(LLVMBuilderRef B, LLVMValueRef Dst,
                                       MaybeAlign(DstAlign), IsVolatile));
 }
 
-// OpBundlesIndirect is an array of pointers (*not* a pointer to an array).
+// Polyfill for `LLVMBuildCallBr`, which was added in LLVM 19.
+// <https://github.com/llvm/llvm-project/commit/584253c4e2f788f870488fc32193b52d67ddaccc>
+// FIXME: Remove when Rust's minimum supported LLVM version reaches 19.
+#if LLVM_VERSION_LT(19, 0)
+DEFINE_SIMPLE_CONVERSION_FUNCTIONS(OperandBundleDef, LLVMOperandBundleRef)
+
 extern "C" LLVMValueRef
-LLVMRustBuildInvoke(LLVMBuilderRef B, LLVMTypeRef Ty, LLVMValueRef Fn,
+LLVMBuildCallBr(LLVMBuilderRef B, LLVMTypeRef Ty, LLVMValueRef Fn,
-                    LLVMValueRef *Args, unsigned NumArgs,
+                LLVMBasicBlockRef DefaultDest, LLVMBasicBlockRef *IndirectDests,
-                    LLVMBasicBlockRef Then, LLVMBasicBlockRef Catch,
+                unsigned NumIndirectDests, LLVMValueRef *Args, unsigned NumArgs,
-                    OperandBundleDef **OpBundlesIndirect, unsigned NumOpBundles,
+                LLVMOperandBundleRef *Bundles, unsigned NumBundles,
-                    const char *Name) {
+                const char *Name) {
-  Value *Callee = unwrap(Fn);
-  FunctionType *FTy = unwrap<FunctionType>(Ty);
-
-  // FIXME: Is there a way around this?
-  SmallVector<OperandBundleDef> OpBundles;
-  OpBundles.reserve(NumOpBundles);
-  for (unsigned i = 0; i < NumOpBundles; ++i) {
-    OpBundles.push_back(*OpBundlesIndirect[i]);
-  }
-
-  return wrap(unwrap(B)->CreateInvoke(FTy, Callee, unwrap(Then), unwrap(Catch),
-                                      ArrayRef<Value *>(unwrap(Args), NumArgs),
-                                      ArrayRef<OperandBundleDef>(OpBundles),
-                                      Name));
-}
-
-// OpBundlesIndirect is an array of pointers (*not* a pointer to an array).
-extern "C" LLVMValueRef
-LLVMRustBuildCallBr(LLVMBuilderRef B, LLVMTypeRef Ty, LLVMValueRef Fn,
-                    LLVMBasicBlockRef DefaultDest,
-                    LLVMBasicBlockRef *IndirectDests, unsigned NumIndirectDests,
-                    LLVMValueRef *Args, unsigned NumArgs,
-                    OperandBundleDef **OpBundlesIndirect, unsigned NumOpBundles,
-                    const char *Name) {
   Value *Callee = unwrap(Fn);
   FunctionType *FTy = unwrap<FunctionType>(Ty);
 
@@ -1639,9 +1588,9 @@ LLVMRustBuildCallBr(LLVMBuilderRef B, LLVMTypeRef Ty, LLVMValueRef Fn,
 
   // FIXME: Is there a way around this?
   SmallVector<OperandBundleDef> OpBundles;
-  OpBundles.reserve(NumOpBundles);
+  OpBundles.reserve(NumBundles);
-  for (unsigned i = 0; i < NumOpBundles; ++i) {
+  for (unsigned i = 0; i < NumBundles; ++i) {
-    OpBundles.push_back(*OpBundlesIndirect[i]);
+    OpBundles.push_back(*unwrap(Bundles[i]));
   }
 
   return wrap(
@@ -1650,6 +1599,7 @@ LLVMRustBuildCallBr(LLVMBuilderRef B, LLVMTypeRef Ty, LLVMValueRef Fn,
                               ArrayRef<Value *>(unwrap(Args), NumArgs),
                               ArrayRef<OperandBundleDef>(OpBundles), Name));
 }
+#endif
 
 extern "C" void LLVMRustPositionBuilderAtStart(LLVMBuilderRef B,
                                                LLVMBasicBlockRef BB) {