Refactor emitting errors to declare.rs, and use fluently-generated error messages

Create lints for deprecated and invalid intrinsics

Author: sayantn

compiler/rustc_codegen_llvm/messages.ftl

@@ -9,6 +9,10 @@ codegen_llvm_from_llvm_diag = {$message}
 
 codegen_llvm_from_llvm_optimization_diag = {$filename}:{$line}:{$column} {$pass_name} ({$kind}): {$message}
 
+codegen_llvm_intrinsic_signature_mismatch =
+    intrinsic signature mismatch for `{$name}`: expected signature `{$llvm_fn_ty}`, found `{$rust_fn_ty}`
+
+
 codegen_llvm_load_bitcode = failed to load bitcode of module "{$name}"
 codegen_llvm_load_bitcode_with_llvm_err = failed to load bitcode of module "{$name}": {$llvm_err}
 

compiler/rustc_codegen_llvm/src/abi.rs

@@ -306,33 +306,42 @@ impl<'ll, 'tcx> ArgAbiBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
 }
 
 pub(crate) enum FunctionSignature<'ll> {
-    /// The signature is obtained directly from LLVM, and **may not match the Rust signature**
-    Intrinsic(&'ll Type),
+    /// This is an LLVM intrinsic, the signature is obtained directly from LLVM, and **may not match the Rust signature**
+    LLVMSignature(llvm::Intrinsic, &'ll Type),
+    /// This is an LLVM intrinsic, but the signature is just the Rust signature.
+    /// FIXME: this should ideally not exist, we should be using the LLVM signature for all LLVM intrinsics
+    RustSignature(llvm::Intrinsic, &'ll Type),
     /// The name starts with `llvm.`, but can't obtain the intrinsic ID. May be invalid or upgradable
-    MaybeInvalidIntrinsic(&'ll Type),
+    MaybeInvalid(&'ll Type),
     /// Just the Rust signature
-    Rust(&'ll Type),
+    NotIntrinsic(&'ll Type),
 }
 
 impl<'ll> FunctionSignature<'ll> {
     pub(crate) fn fn_ty(&self) -> &'ll Type {
         match self {
-            FunctionSignature::Intrinsic(fn_ty)
-            | FunctionSignature::MaybeInvalidIntrinsic(fn_ty)
-            | FunctionSignature::Rust(fn_ty) => fn_ty,
+            FunctionSignature::LLVMSignature(_, fn_ty)
+            | FunctionSignature::RustSignature(_, fn_ty)
+            | FunctionSignature::MaybeInvalid(fn_ty)
+            | FunctionSignature::NotIntrinsic(fn_ty) => fn_ty,
+        }
+    }
+
+    pub(crate) fn intrinsic(&self) -> Option<llvm::Intrinsic> {
+        match self {
+            FunctionSignature::RustSignature(intrinsic, _)
+            | FunctionSignature::LLVMSignature(intrinsic, _) => Some(*intrinsic),
+            _ => None,
         }
     }
 }
 
 pub(crate) trait FnAbiLlvmExt<'ll, 'tcx> {
     fn llvm_return_type(&self, cx: &CodegenCx<'ll, 'tcx>) -> &'ll Type;
     fn llvm_argument_types(&self, cx: &CodegenCx<'ll, 'tcx>) -> Vec<&'ll Type>;
-    fn llvm_type(
-        &self,
-        cx: &CodegenCx<'ll, 'tcx>,
-        name: &[u8],
-        do_verify: bool,
-    ) -> FunctionSignature<'ll>;
+    fn llvm_type(&self, cx: &CodegenCx<'ll, 'tcx>, name: &[u8]) -> FunctionSignature<'ll>;
+    /// The normal Rust signature for this
+    fn rust_signature(&self, cx: &CodegenCx<'ll, 'tcx>) -> &'ll Type;
     /// **If this function is an LLVM intrinsic** checks if the LLVM signature provided matches with this
     fn verify_intrinsic_signature(&self, cx: &CodegenCx<'ll, 'tcx>, llvm_ty: &'ll Type) -> bool;
 
@@ -478,51 +487,35 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
             .all(|(rust_ty, llvm_ty)| cx.equate_ty(rust_ty, llvm_ty))
     }
 
-    fn llvm_type(
-        &self,
-        cx: &CodegenCx<'ll, 'tcx>,
-        name: &[u8],
-        do_verify: bool,
-    ) -> FunctionSignature<'ll> {
-        let mut maybe_invalid = false;
+    fn rust_signature(&self, cx: &CodegenCx<'ll, 'tcx>) -> &'ll Type {
+        let return_ty = self.llvm_return_type(cx);
+        let argument_tys = self.llvm_argument_types(cx);
 
+        if self.c_variadic {
+            cx.type_variadic_func(&argument_tys, return_ty)
+        } else {
+            cx.type_func(&argument_tys, return_ty)
+        }
+    }
+
+    fn llvm_type(&self, cx: &CodegenCx<'ll, 'tcx>, name: &[u8]) -> FunctionSignature<'ll> {
         if name.starts_with(b"llvm.") {
             if let Some(intrinsic) = llvm::Intrinsic::lookup(name) {
                 if !intrinsic.is_overloaded() {
                     // FIXME: also do this for overloaded intrinsics
-                    let llvm_fn_ty = intrinsic.get_type(cx.llcx, &[]);
-                    if do_verify {
-                        if !self.verify_intrinsic_signature(cx, llvm_fn_ty) {
-                            cx.tcx.dcx().fatal(format!(
-                                "Intrinsic signature mismatch for `{}`: expected signature `{llvm_fn_ty:?}`",
-                                str::from_utf8(name).unwrap()
-                            ));
-                        }
-                    }
-                    return FunctionSignature::Intrinsic(llvm_fn_ty);
+                    FunctionSignature::LLVMSignature(intrinsic, intrinsic.get_type(cx.llcx, &[]))
+                } else {
+                    FunctionSignature::RustSignature(intrinsic, self.rust_signature(cx))
                 }
             } else {
                 // it's one of 2 cases,
                 // - either the base name is invalid
                 // - it has been superseded by something else, so the intrinsic was removed entirely
                 // to check for upgrades, we need the `llfn`, so we defer it for now
-                maybe_invalid = true;
+                FunctionSignature::MaybeInvalid(self.rust_signature(cx))
             }
-        }
-
-        let return_ty = self.llvm_return_type(cx);
-        let argument_tys = self.llvm_argument_types(cx);
-
-        let fn_ty = if self.c_variadic {
-            cx.type_variadic_func(&argument_tys, return_ty)
         } else {
-            cx.type_func(&argument_tys, return_ty)
-        };
-
-        if maybe_invalid {
-            FunctionSignature::MaybeInvalidIntrinsic(fn_ty)
-        } else {
-            FunctionSignature::Rust(fn_ty)
+            FunctionSignature::NotIntrinsic(self.rust_signature(cx))
         }
     }
 
@@ -686,15 +679,9 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
         callsite: &'ll Value,
         llfn: &'ll Value,
     ) {
-        // if we are using the LLVM signature, use the LLVM attributes otherwise it might be problematic
-        let name = llvm::get_value_name(llfn);
-        if name.starts_with(b"llvm.")
-            && let Some(intrinsic) = llvm::Intrinsic::lookup(&name)
-        {
-            // FIXME: also do this for overloaded intrinsics
-            if !intrinsic.is_overloaded() {
-                return;
-            }
+        // Don't apply any attributes to LLVM intrinsics, they will be applied by AutoUpgrade
+        if llvm::get_value_name(llfn).starts_with(b"llvm.") {
+            return;
         }
 
         let mut func_attrs = SmallVec::<[_; 2]>::new();

compiler/rustc_codegen_llvm/src/declare.rs

@@ -18,6 +18,7 @@ use rustc_codegen_ssa::traits::TypeMembershipCodegenMethods;
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_middle::ty::{Instance, Ty};
 use rustc_sanitizers::{cfi, kcfi};
+use rustc_session::lint::builtin::{DEPRECATED_LLVM_INTRINSIC, UNKNOWN_LLVM_INTRINSIC};
 use rustc_target::callconv::FnAbi;
 use smallvec::SmallVec;
 use tracing::debug;
@@ -29,7 +30,7 @@ use crate::llvm::AttributePlace::Function;
 use crate::llvm::{FromGeneric, Visibility};
 use crate::type_::Type;
 use crate::value::Value;
-use crate::{attributes, llvm};
+use crate::{attributes, errors, llvm};
 
 /// Declare a function with a SimpleCx.
 ///
@@ -150,52 +151,69 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
     ) -> &'ll Value {
         debug!("declare_rust_fn(name={:?}, fn_abi={:?})", name, fn_abi);
 
-        let signature = fn_abi.llvm_type(self, name.as_bytes(), true);
-        let llfn;
-
-        if let FunctionSignature::Intrinsic(fn_ty) = signature {
-            // intrinsics have a specified set of attributes, so we don't use the `FnAbi` set for them
-            llfn = declare_simple_fn(
-                self,
-                name,
-                fn_abi.llvm_cconv(self),
-                llvm::UnnamedAddr::Global,
-                llvm::Visibility::Default,
-                fn_ty,
-            );
-        } else {
-            // Function addresses in Rust are never significant, allowing functions to
-            // be merged.
-            llfn = declare_raw_fn(
-                self,
-                name,
-                fn_abi.llvm_cconv(self),
-                llvm::UnnamedAddr::Global,
-                llvm::Visibility::Default,
-                signature.fn_ty(),
-            );
+        let signature = fn_abi.llvm_type(self, name.as_bytes());
+
+        let span = || instance.map(|instance| self.tcx.def_span(instance.def_id()));
+
+        if let FunctionSignature::LLVMSignature(_, llvm_fn_ty) = signature {
+            // check if the intrinsic signatures match
+            if !fn_abi.verify_intrinsic_signature(self, llvm_fn_ty) {
+                self.tcx.dcx().emit_fatal(errors::IntrinsicSignatureMismatch {
+                    name,
+                    llvm_fn_ty: &format!("{llvm_fn_ty:?}"),
+                    rust_fn_ty: &format!("{:?}", fn_abi.rust_signature(self)),
+                    span: span(),
+                });
+            }
+        }
+
+        // Function addresses in Rust are never significant, allowing functions to
+        // be merged.
+        let llfn = declare_raw_fn(
+            self,
+            name,
+            fn_abi.llvm_cconv(self),
+            llvm::UnnamedAddr::Global,
+            llvm::Visibility::Default,
+            signature.fn_ty(),
+        );
+
+        if signature.intrinsic().is_none() {
+            // Don't apply any attributes to intrinsics, they will be applied by AutoUpgrade
             fn_abi.apply_attrs_llfn(self, llfn, instance);
         }
 
-        if let FunctionSignature::MaybeInvalidIntrinsic(..) = signature {
+        if let FunctionSignature::MaybeInvalid(..) = signature {
             let mut new_llfn = None;
             let can_upgrade =
                 unsafe { llvm::LLVMRustUpgradeIntrinsicFunction(llfn, &mut new_llfn, false) };
 
-            if can_upgrade {
-                // not all intrinsics are upgraded to some other intrinsics, most are upgraded to instruction sequences
-                if let Some(new_llfn) = new_llfn {
-                    self.tcx.dcx().note(format!(
-                        "Using deprecated intrinsic `{name}`, `{}` can be used instead",
-                        str::from_utf8(&llvm::get_value_name(new_llfn)).unwrap()
-                    ));
+            // we can emit diagnostics for local crates only
+            if let Some(instance) = instance
+                && let Some(local_def_id) = instance.def_id().as_local()
+            {
+                let hir_id = self.tcx.local_def_id_to_hir_id(local_def_id);
+                let span = self.tcx.def_span(local_def_id);
+
+                if can_upgrade {
+                    // not all intrinsics are upgraded to some other intrinsics, most are upgraded to instruction sequences
+                    let msg = if let Some(new_llfn) = new_llfn {
+                        format!(
+                            "Using deprecated intrinsic `{name}`, `{}` can be used instead",
+                            str::from_utf8(&llvm::get_value_name(new_llfn)).unwrap()
+                        )
+                    } else {
+                        format!("Using deprecated intrinsic `{name}`")
+                    };
+                    self.tcx.node_lint(DEPRECATED_LLVM_INTRINSIC, hir_id, |d| {
+                        d.primary_message(msg).span(span);
+                    });
                 } else {
-                    self.tcx.dcx().note(format!(
-                        "Using deprecated intrinsic `{name}`, consider using other intrinsics/instructions"
-                    ));
+                    // This is either plain wrong, or this can be caused by incompatible LLVM versions, we let the user decide
+                    self.tcx.node_lint(UNKNOWN_LLVM_INTRINSIC, hir_id, |d| {
+                        d.primary_message(format!("Invalid LLVM Intrinsic `{name}`")).span(span);
+                    });
                 }
-            } else {
-                self.tcx.dcx().fatal(format!("Invalid LLVM intrinsic: `{name}`"))
             }
         }
 

compiler/rustc_codegen_llvm/src/errors.rs

@@ -147,3 +147,13 @@ pub(crate) struct FixedX18InvalidArch<'a> {
 #[derive(Diagnostic)]
 #[diag(codegen_llvm_sanitizer_kcfi_arity_requires_llvm_21_0_0)]
 pub(crate) struct SanitizerKcfiArityRequiresLLVM2100;
+
+#[derive(Diagnostic)]
+#[diag(codegen_llvm_intrinsic_signature_mismatch)]
+pub(crate) struct IntrinsicSignatureMismatch<'a> {
+    pub name: &'a str,
+    pub llvm_fn_ty: &'a str,
+    pub rust_fn_ty: &'a str,
+    #[primary_span]
+    pub span: Option<Span>,
+}

compiler/rustc_codegen_llvm/src/intrinsic.rs

@@ -1075,7 +1075,7 @@ fn gen_fn<'a, 'll, 'tcx>(
     codegen: &mut dyn FnMut(Builder<'a, 'll, 'tcx>),
 ) -> (&'ll Type, &'ll Value) {
     let fn_abi = cx.fn_abi_of_fn_ptr(rust_fn_sig, ty::List::empty());
-    let llty = fn_abi.llvm_type(cx, name.as_bytes(), true).fn_ty();
+    let llty = fn_abi.llvm_type(cx, name.as_bytes()).fn_ty();
     let llfn = cx.declare_fn(name, fn_abi, None);
     cx.set_frame_pointer_type(llfn);
     cx.apply_target_cpu_attr(llfn);

compiler/rustc_codegen_llvm/src/type_.rs

@@ -311,7 +311,7 @@ impl<'ll, 'tcx> LayoutTypeCodegenMethods<'tcx> for CodegenCx<'ll, 'tcx> {
         fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
         fn_ptr: &'ll Value,
     ) -> &'ll Type {
-        fn_abi.llvm_type(self, &llvm::get_value_name(fn_ptr), false).fn_ty()
+        fn_abi.llvm_type(self, &llvm::get_value_name(fn_ptr)).fn_ty()
     }
     fn fn_ptr_backend_type(&self, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> &'ll Type {
         fn_abi.ptr_to_llvm_type(self)