Skip to content

Commit 35d0f9d

Browse files
sayantnsayantn
committed
Refactor emitting errors to declare.rs, and use fluently-generated error messages
1 parent 602999c commit 35d0f9d

12 files changed

+148
-99
lines changed

‎compiler/rustc_codegen_llvm/messages.ftl

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,26 @@ codegen_llvm_autodiff_without_enable = using the autodiff feature requires -Z au
22
33
codegen_llvm_copy_bitcode = failed to copy bitcode to object file: {$err}
44
5+
codegen_llvm_deprecated_intrinsic =
6+
Using deprecated intrinsic `{$name}`, consider using other intrinsics/instructions
7+
8+
codegen_llvm_deprecated_intrinsic_with_replacement =
9+
Using deprecated intrinsic `{$name}`, `{$replacement}` can be used instead
10+
11+
512
613
codegen_llvm_fixed_x18_invalid_arch = the `-Zfixed-x18` flag is not supported on the `{$arch}` architecture
714
815
codegen_llvm_from_llvm_diag = {$message}
916
1017
codegen_llvm_from_llvm_optimization_diag = {$filename}:{$line}:{$column} {$pass_name} ({$kind}): {$message}
1118
19+
codegen_llvm_intrinsic_signature_mismatch =
20+
Intrinsic signature mismatch for `{$name}`: expected signature `{$llvm_fn_ty}`, found `{$rust_fn_ty}`
21+
22+
codegen_llvm_invalid_intrinsic =
23+
Invalid LLVM Intrinsic `{$name}`
24+
1225
codegen_llvm_load_bitcode = failed to load bitcode of module "{$name}"
1326
codegen_llvm_load_bitcode_with_llvm_err = failed to load bitcode of module "{$name}": {$llvm_err}
1427

‎compiler/rustc_codegen_llvm/src/abi.rs

Lines changed: 41 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -308,20 +308,32 @@ impl<'ll, 'tcx> ArgAbiBuilderMethods<'tcx> for Builder<'_, 'll, 'tcx> {
308308
}
309309

310310
pub(crate) enum FunctionSignature<'ll> {
311-
/// The signature is obtained directly from LLVM, and **may not match the Rust signature**
312-
Intrinsic(&'ll Type),
311+
/// This is an LLVM intrinsic, the signature is obtained directly from LLVM, and **may not match the Rust signature**
312+
LLVMSignature(llvm::Intrinsic, &'ll Type),
313+
/// This is an LLVM intrinsic, but the signature is just the Rust signature.
314+
/// FIXME: this should ideally not exist, we should be using the LLVM signature for all LLVM intrinsics
315+
RustSignature(llvm::Intrinsic, &'ll Type),
313316
/// The name starts with `llvm.`, but can't obtain the intrinsic ID. May be invalid or upgradable
314-
MaybeInvalidIntrinsic(&'ll Type),
317+
MaybeInvalid(&'ll Type),
315318
/// Just the Rust signature
316-
Rust(&'ll Type),
319+
NotIntrinsic(&'ll Type),
317320
}
318321

319322
impl<'ll> FunctionSignature<'ll> {
320323
pub(crate) fn fn_ty(&self) -> &'ll Type {
321324
match self {
322-
FunctionSignature::Intrinsic(fn_ty)
323-
| FunctionSignature::MaybeInvalidIntrinsic(fn_ty)
324-
| FunctionSignature::Rust(fn_ty) => fn_ty,
325+
FunctionSignature::LLVMSignature(_, fn_ty)
326+
| FunctionSignature::RustSignature(_, fn_ty)
327+
| FunctionSignature::MaybeInvalid(fn_ty)
328+
| FunctionSignature::NotIntrinsic(fn_ty) => fn_ty,
329+
}
330+
}
331+
332+
pub(crate) fn intrinsic(&self) -> Option<llvm::Intrinsic> {
333+
match self {
334+
FunctionSignature::RustSignature(intrinsic, _)
335+
| FunctionSignature::LLVMSignature(intrinsic, _) => Some(*intrinsic),
336+
_ => None,
325337
}
326338
}
327339
}
@@ -332,12 +344,9 @@ pub(crate) trait FnAbiLlvmExt<'ll, 'tcx> {
332344
/// When `do_verify` is set, this function performs checks for the signature of LLVM intrinsics
333345
/// and emits a fatal error if it doesn't match. These checks are important,but somewhat expensive
334346
/// So they are only used at function definitions, not at callsites
335-
fn llvm_type(
336-
&self,
337-
cx: &CodegenCx<'ll, 'tcx>,
338-
name: &[u8],
339-
do_verify: bool,
340-
) -> FunctionSignature<'ll>;
347+
fn llvm_type(&self, cx: &CodegenCx<'ll, 'tcx>, name: &[u8]) -> FunctionSignature<'ll>;
348+
/// The normal Rust signature for this
349+
fn rust_signature(&self, cx: &CodegenCx<'ll, 'tcx>) -> &'ll Type;
341350
/// **If this function is an LLVM intrinsic** checks if the LLVM signature provided matches with this
342351
fn verify_intrinsic_signature(&self, cx: &CodegenCx<'ll, 'tcx>, llvm_ty: &'ll Type) -> bool;
343352
fn ptr_to_llvm_type(&self, cx: &CodegenCx<'ll, 'tcx>) -> &'ll Type;
@@ -481,52 +490,36 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
481490
.all(|(rust_ty, llvm_ty)| cx.equate_ty(rust_ty, llvm_ty))
482491
}
483492

484-
fn llvm_type(
485-
&self,
486-
cx: &CodegenCx<'ll, 'tcx>,
487-
name: &[u8],
488-
do_verify: bool,
489-
) -> FunctionSignature<'ll> {
490-
let mut maybe_invalid = false;
493+
fn rust_signature(&self, cx: &CodegenCx<'ll, 'tcx>) -> &'ll Type {
494+
let return_ty = self.llvm_return_type(cx);
495+
let argument_tys = self.llvm_argument_types(cx);
496+
497+
if self.c_variadic {
498+
cx.type_variadic_func(&argument_tys, return_ty)
499+
} else {
500+
cx.type_func(&argument_tys, return_ty)
501+
}
502+
}
491503

504+
fn llvm_type(&self, cx: &CodegenCx<'ll, 'tcx>, name: &[u8]) -> FunctionSignature<'ll> {
492505
if name.starts_with(b"llvm.") {
493506
if let Some(intrinsic) = llvm::Intrinsic::lookup(name) {
494507
if !intrinsic.is_overloaded() {
495508
// FIXME: also do this for overloaded intrinsics
496-
let llvm_fn_ty = intrinsic.get_type(cx.llcx, &[]);
497-
if do_verify {
498-
if !self.verify_intrinsic_signature(cx, llvm_fn_ty) {
499-
cx.tcx.dcx().fatal(format!(
500-
"Intrinsic signature mismatch for `{}`: expected signature `{llvm_fn_ty:?}`",
501-
str::from_utf8(name).unwrap()
502-
));
503-
}
504-
}
505-
return FunctionSignature::Intrinsic(llvm_fn_ty);
509+
FunctionSignature::LLVMSignature(intrinsic, intrinsic.get_type(cx.llcx, &[]))
510+
} else {
511+
FunctionSignature::RustSignature(intrinsic, self.rust_signature(cx))
506512
}
507513
} else {
508514
// it's one of 2 cases,
509515
// - either the base name is invalid
510516
// - it has been superseded by something else, so the intrinsic was removed entirely
511517
// to check for upgrades, we need the `llfn`, so we defer it for now
512518

513-
maybe_invalid = true;
519+
FunctionSignature::MaybeInvalid(self.rust_signature(cx))
514520
}
515-
}
516-
517-
let return_ty = self.llvm_return_type(cx);
518-
let argument_tys = self.llvm_argument_types(cx);
519-
520-
let fn_ty = if self.c_variadic {
521-
cx.type_variadic_func(&argument_tys, return_ty)
522521
} else {
523-
cx.type_func(&argument_tys, return_ty)
524-
};
525-
526-
if maybe_invalid {
527-
FunctionSignature::MaybeInvalidIntrinsic(fn_ty)
528-
} else {
529-
FunctionSignature::Rust(fn_ty)
522+
FunctionSignature::NotIntrinsic(self.rust_signature(cx))
530523
}
531524
}
532525

@@ -684,15 +677,9 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
684677
callsite: &'ll Value,
685678
llfn: &'ll Value,
686679
) {
687-
// if we are using the LLVM signature, use the LLVM attributes otherwise it might be problematic
688-
let name = llvm::get_value_name(llfn);
689-
if name.starts_with(b"llvm.")
690-
&& let Some(intrinsic) = llvm::Intrinsic::lookup(&name)
691-
{
692-
// FIXME: also do this for overloaded intrinsics
693-
if !intrinsic.is_overloaded() {
694-
return;
695-
}
680+
// Don't apply any attributes to LLVM intrinsics, they will be applied by AutoUpgrade
681+
if llvm::get_value_name(llfn).starts_with(b"llvm.") {
682+
return;
696683
}
697684

698685
let mut func_attrs = SmallVec::<[_; 2]>::new();

‎compiler/rustc_codegen_llvm/src/declare.rs

Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use crate::llvm::AttributePlace::Function;
2929
use crate::llvm::Visibility;
3030
use crate::type_::Type;
3131
use crate::value::Value;
32-
use crate::{attributes, llvm};
32+
use crate::{attributes, errors, llvm};
3333

3434
/// Declare a function with a SimpleCx.
3535
///
@@ -150,52 +150,59 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
150150
) -> &'ll Value {
151151
debug!("declare_rust_fn(name={:?}, fn_abi={:?})", name, fn_abi);
152152

153-
let signature = fn_abi.llvm_type(self, name.as_bytes(), true);
154-
let llfn;
153+
let signature = fn_abi.llvm_type(self, name.as_bytes());
155154

156-
if let FunctionSignature::Intrinsic(fn_ty) = signature {
157-
// intrinsics have a specified set of attributes, so we don't use the `FnAbi` set for them
158-
llfn = declare_simple_fn(
159-
self,
160-
name,
161-
fn_abi.llvm_cconv(self),
162-
llvm::UnnamedAddr::Global,
163-
llvm::Visibility::Default,
164-
fn_ty,
165-
);
166-
} else {
167-
// Function addresses in Rust are never significant, allowing functions to
168-
// be merged.
169-
llfn = declare_raw_fn(
170-
self,
171-
name,
172-
fn_abi.llvm_cconv(self),
173-
llvm::UnnamedAddr::Global,
174-
llvm::Visibility::Default,
175-
signature.fn_ty(),
176-
);
155+
let span = || instance.map(|instance| self.tcx.def_span(instance.def_id()));
156+
157+
if let FunctionSignature::LLVMSignature(_, llvm_fn_ty) = signature {
158+
// check if the intrinsic signatures match
159+
if !fn_abi.verify_intrinsic_signature(self, llvm_fn_ty) {
160+
self.tcx.dcx().emit_fatal(errors::IntrinsicSignatureMismatch {
161+
name,
162+
llvm_fn_ty: &format!("{llvm_fn_ty:?}"),
163+
rust_fn_ty: &format!("{:?}", fn_abi.rust_signature(self)),
164+
span: span(),
165+
});
166+
}
167+
}
168+
169+
// Function addresses in Rust are never significant, allowing functions to
170+
// be merged.
171+
let llfn = declare_raw_fn(
172+
self,
173+
name,
174+
fn_abi.llvm_cconv(self),
175+
llvm::UnnamedAddr::Global,
176+
llvm::Visibility::Default,
177+
signature.fn_ty(),
178+
);
179+
180+
if signature.intrinsic().is_none() {
181+
// Don't apply any attributes to intrinsics, they will be applied by AutoUpgrade
177182
fn_abi.apply_attrs_llfn(self, llfn, instance);
178183
}
179184

180-
if let FunctionSignature::MaybeInvalidIntrinsic(..) = signature {
185+
if let FunctionSignature::MaybeInvalid(..) = signature {
181186
let mut new_llfn = None;
182187
let can_upgrade =
183188
unsafe { llvm::LLVMRustUpgradeIntrinsicFunction(llfn, &mut new_llfn, false) };
184189

185190
if can_upgrade {
186191
// not all intrinsics are upgraded to some other intrinsics, most are upgraded to instruction sequences
187192
if let Some(new_llfn) = new_llfn {
188-
self.tcx.dcx().note(format!(
189-
"Using deprecated intrinsic `{name}`, `{}` can be used instead",
190-
str::from_utf8(&llvm::get_value_name(new_llfn)).unwrap()
191-
));
193+
self.tcx.dcx().emit_note(errors::DeprecatedIntrinsicWithReplacement {
194+
name,
195+
replacement: str::from_utf8(&llvm::get_value_name(new_llfn)).unwrap(),
196+
span: span(),
197+
});
192198
} else if self.tcx.sess.opts.verbose {
193-
self.tcx.dcx().note(format!(
194-
"Using deprecated intrinsic `{name}`, consider using other intrinsics/instructions"
195-
));
199+
// At least for now, we are only emitting notes for deprecated intrinsics with no direct replacement
200+
// because they are used quite a lot in stdarch. After the stdarch uses has been removed, we can make
201+
// this always emit a note (or even an warning)
202+
self.tcx.dcx().emit_note(errors::DeprecatedIntrinsic { name, span: span() });
196203
}
197204
} else {
198-
self.tcx.dcx().fatal(format!("Invalid LLVM intrinsic: `{name}`"))
205+
self.tcx.dcx().emit_fatal(errors::InvalidIntrinsic { name, span: span() });
199206
}
200207
}
201208

‎compiler/rustc_codegen_llvm/src/errors.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,3 +147,38 @@ pub(crate) struct FixedX18InvalidArch<'a> {
147147
#[derive(Diagnostic)]
148148
#[diag(codegen_llvm_sanitizer_kcfi_arity_requires_llvm_21_0_0)]
149149
pub(crate) struct SanitizerKcfiArityRequiresLLVM2100;
150+
151+
#[derive(Diagnostic)]
152+
#[diag(codegen_llvm_intrinsic_signature_mismatch)]
153+
pub(crate) struct IntrinsicSignatureMismatch<'a> {
154+
pub name: &'a str,
155+
pub llvm_fn_ty: &'a str,
156+
pub rust_fn_ty: &'a str,
157+
#[primary_span]
158+
pub span: Option<Span>,
159+
}
160+
161+
#[derive(Diagnostic)]
162+
#[diag(codegen_llvm_invalid_intrinsic)]
163+
pub(crate) struct InvalidIntrinsic<'a> {
164+
pub name: &'a str,
165+
#[primary_span]
166+
pub span: Option<Span>,
167+
}
168+
169+
#[derive(Diagnostic)]
170+
#[diag(codegen_llvm_deprecated_intrinsic)]
171+
pub(crate) struct DeprecatedIntrinsic<'a> {
172+
pub name: &'a str,
173+
#[primary_span]
174+
pub span: Option<Span>,
175+
}
176+
177+
#[derive(Diagnostic)]
178+
#[diag(codegen_llvm_deprecated_intrinsic_with_replacement)]
179+
pub(crate) struct DeprecatedIntrinsicWithReplacement<'a> {
180+
pub name: &'a str,
181+
pub replacement: &'a str,
182+
#[primary_span]
183+
pub span: Option<Span>,
184+
}

‎compiler/rustc_codegen_llvm/src/intrinsic.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1066,7 +1066,7 @@ fn gen_fn<'a, 'll, 'tcx>(
10661066
codegen: &mut dyn FnMut(Builder<'a, 'll, 'tcx>),
10671067
) -> (&'ll Type, &'ll Value) {
10681068
let fn_abi = cx.fn_abi_of_fn_ptr(rust_fn_sig, ty::List::empty());
1069-
let llty = fn_abi.llvm_type(cx, name.as_bytes(), true).fn_ty();
1069+
let llty = fn_abi.llvm_type(cx, name.as_bytes()).fn_ty();
10701070
let llfn = cx.declare_fn(name, fn_abi, None);
10711071
cx.set_frame_pointer_type(llfn);
10721072
cx.apply_target_cpu_attr(llfn);

‎compiler/rustc_codegen_llvm/src/type_.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ impl<'ll, 'tcx> LayoutTypeCodegenMethods<'tcx> for CodegenCx<'ll, 'tcx> {
313313
fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
314314
fn_ptr: &'ll Value,
315315
) -> &'ll Type {
316-
fn_abi.llvm_type(self, &llvm::get_value_name(fn_ptr), false).fn_ty()
316+
fn_abi.llvm_type(self, &llvm::get_value_name(fn_ptr)).fn_ty()
317317
}
318318
fn fn_ptr_backend_type(&self, fn_abi: &FnAbi<'tcx, Ty<'tcx>>) -> &'ll Type {
319319
fn_abi.ptr_to_llvm_type(self)

‎tests/ui/codegen/deprecated-llvm-intrinsic.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,10 @@ pub struct i8x8([i8; 8]);
1818
extern "unadjusted" {
1919
#[link_name = "llvm.aarch64.neon.rbit.v8i8"]
2020
fn foo(a: i8x8) -> i8x8;
21+
//~^ NOTE: Using deprecated intrinsic `llvm.aarch64.neon.rbit.v8i8`, `llvm.bitreverse.v8i8` can be used instead
2122
}
2223

2324
#[target_feature(enable = "neon")]
2425
pub unsafe fn bar(a: i8x8) -> i8x8 {
2526
foo(a)
2627
}
27-
28-
//~? NOTE: Using deprecated intrinsic `llvm.aarch64.neon.rbit.v8i8`, `llvm.bitreverse.v8i8` can be used instead

‎tests/ui/codegen/deprecated-llvm-intrinsic.stderr

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,6 @@
11
note: Using deprecated intrinsic `llvm.aarch64.neon.rbit.v8i8`, `llvm.bitreverse.v8i8` can be used instead
2+
--> $DIR/deprecated-llvm-intrinsic.rs:20:5
3+
|
4+
LL | fn foo(a: i8x8) -> i8x8;
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^
26

‎tests/ui/codegen/incorrect-llvm-intrinsic-signature.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
//@ build-fail
22

33
#![feature(link_llvm_intrinsics, abi_unadjusted)]
4-
#![allow(internal_features, non_camel_case_types, improper_ctypes)]
54

65
extern "unadjusted" {
76
#[link_name = "llvm.assume"]
87
fn foo();
8+
//~^ ERROR: Intrinsic signature mismatch for `llvm.assume`: expected signature `void (i1)`, found `void ()`
99
}
1010

1111
pub fn main() {
1212
unsafe { foo() }
1313
}
14-
15-
//~? ERROR: Intrinsic signature mismatch for `llvm.assume`: expected signature `void (i1)`

‎tests/ui/codegen/incorrect-llvm-intrinsic-signature.stderr

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
error: Intrinsic signature mismatch for `llvm.assume`: expected signature `void (i1)`
1+
error: Intrinsic signature mismatch for `llvm.assume`: expected signature `void (i1)`, found `void ()`
2+
--> $DIR/incorrect-llvm-intrinsic-signature.rs:7:5
3+
|
4+
LL | fn foo();
5+
| ^^^^^^^^^
26

37
error: aborting due to 1 previous error
48

0 commit comments

Comments
 (0)