From e9ea57814768cdae0b36065a731009afa7e36f31 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 30 Mar 2024 12:01:57 +0000 Subject: [PATCH 01/19] Move vcall_visibility_metadata optimization hint out of a debuginfo generation method --- compiler/rustc_codegen_llvm/src/context.rs | 10 ++++++++++ .../rustc_codegen_llvm/src/debuginfo/metadata.rs | 14 +++++++------- compiler/rustc_codegen_ssa/src/meth.rs | 1 + compiler/rustc_codegen_ssa/src/traits/misc.rs | 7 +++++++ 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index 7d92888feeed4..57682d1cc8c4c 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -3,6 +3,7 @@ use crate::back::write::to_llvm_code_model; use crate::callee::get_fn; use crate::coverageinfo; use crate::debuginfo; +use crate::debuginfo::metadata::apply_vcall_visibility_metadata; use crate::llvm; use crate::llvm_util; use crate::type_::Type; @@ -522,6 +523,15 @@ impl<'ll, 'tcx> MiscMethods<'tcx> for CodegenCx<'ll, 'tcx> { &self.vtables } + fn apply_vcall_visibility_metadata( + &self, + ty: Ty<'tcx>, + poly_trait_ref: Option>, + vtable: &'ll Value, + ) { + apply_vcall_visibility_metadata(self, ty, poly_trait_ref, vtable); + } + fn get_fn(&self, instance: Instance<'tcx>) -> &'ll Value { get_fn(self, instance) } diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index 8de4e0effad28..742bfd76590a4 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -1449,12 +1449,18 @@ fn build_vtable_type_di_node<'ll, 'tcx>( .di_node } -fn vcall_visibility_metadata<'ll, 'tcx>( +pub(crate) fn apply_vcall_visibility_metadata<'ll, 'tcx>( cx: &CodegenCx<'ll, 'tcx>, ty: Ty<'tcx>, trait_ref: Option>, vtable: &'ll Value, ) { + // FIXME(flip1995): The virtual function elimination optimization only works with full LTO in + // LLVM at the moment. + if !cx.sess().opts.unstable_opts.virtual_function_elimination || cx.sess().lto() != Lto::Fat { + return; + } + enum VCallVisibility { Public = 0, LinkageUnit = 1, @@ -1531,12 +1537,6 @@ pub fn create_vtable_di_node<'ll, 'tcx>( poly_trait_ref: Option>, vtable: &'ll Value, ) { - // FIXME(flip1995): The virtual function elimination optimization only works with full LTO in - // LLVM at the moment. - if cx.sess().opts.unstable_opts.virtual_function_elimination && cx.sess().lto() == Lto::Fat { - vcall_visibility_metadata(cx, ty, poly_trait_ref, vtable); - } - if cx.dbg_cx.is_none() { return; } diff --git a/compiler/rustc_codegen_ssa/src/meth.rs b/compiler/rustc_codegen_ssa/src/meth.rs index ddc6797388e77..febc8ee2be248 100644 --- a/compiler/rustc_codegen_ssa/src/meth.rs +++ b/compiler/rustc_codegen_ssa/src/meth.rs @@ -133,6 +133,7 @@ pub fn get_vtable<'tcx, Cx: CodegenMethods<'tcx>>( let align = cx.data_layout().pointer_align.abi; let vtable = cx.static_addr_of(vtable_const, align, Some("vtable")); + cx.apply_vcall_visibility_metadata(ty, trait_ref, vtable); cx.create_vtable_debuginfo(ty, trait_ref, vtable); cx.vtables().borrow_mut().insert((ty, trait_ref), vtable); vtable diff --git a/compiler/rustc_codegen_ssa/src/traits/misc.rs b/compiler/rustc_codegen_ssa/src/traits/misc.rs index 04e2b8796c46a..af3a998960491 100644 --- a/compiler/rustc_codegen_ssa/src/traits/misc.rs +++ b/compiler/rustc_codegen_ssa/src/traits/misc.rs @@ -9,6 +9,13 @@ pub trait MiscMethods<'tcx>: BackendTypes { fn vtables( &self, ) -> &RefCell, Option>), Self::Value>>; + fn apply_vcall_visibility_metadata( + &self, + _ty: Ty<'tcx>, + _poly_trait_ref: Option>, + _vtable: Self::Value, + ) { + } fn check_overflow(&self) -> bool; fn get_fn(&self, instance: Instance<'tcx>) -> Self::Function; fn get_fn_addr(&self, instance: Instance<'tcx>) -> Self::Value; From 7f445329ec3330f4334431b68b26325922391e5c Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 30 Mar 2024 13:19:58 +0000 Subject: [PATCH 02/19] Remove PrintBackendInfo trait It is only implemented for a single type. Directly passing this type is simpler and avoids overhead from indirect calls. --- compiler/rustc_codegen_llvm/src/lib.rs | 24 ++++++++------- compiler/rustc_codegen_llvm/src/llvm_util.rs | 30 ++++++++++--------- .../rustc_codegen_ssa/src/traits/backend.rs | 20 +------------ compiler/rustc_codegen_ssa/src/traits/mod.rs | 4 +-- 4 files changed, 31 insertions(+), 47 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs index 4b7a264300711..ed0989a0ba413 100644 --- a/compiler/rustc_codegen_llvm/src/lib.rs +++ b/compiler/rustc_codegen_llvm/src/lib.rs @@ -274,10 +274,11 @@ impl CodegenBackend for LlvmCodegenBackend { |tcx, ()| llvm_util::global_llvm_features(tcx.sess, true) } - fn print(&self, req: &PrintRequest, out: &mut dyn PrintBackendInfo, sess: &Session) { + fn print(&self, req: &PrintRequest, out: &mut String, sess: &Session) { + use std::fmt::Write; match req.kind { PrintKind::RelocationModels => { - writeln!(out, "Available relocation models:"); + writeln!(out, "Available relocation models:").unwrap(); for name in &[ "static", "pic", @@ -288,25 +289,25 @@ impl CodegenBackend for LlvmCodegenBackend { "ropi-rwpi", "default", ] { - writeln!(out, " {name}"); + writeln!(out, " {name}").unwrap(); } - writeln!(out); + writeln!(out).unwrap(); } PrintKind::CodeModels => { - writeln!(out, "Available code models:"); + writeln!(out, "Available code models:").unwrap(); for name in &["tiny", "small", "kernel", "medium", "large"] { - writeln!(out, " {name}"); + writeln!(out, " {name}").unwrap(); } - writeln!(out); + writeln!(out).unwrap(); } PrintKind::TlsModels => { - writeln!(out, "Available TLS models:"); + writeln!(out, "Available TLS models:").unwrap(); for name in &["global-dynamic", "local-dynamic", "initial-exec", "local-exec", "emulated"] { - writeln!(out, " {name}"); + writeln!(out, " {name}").unwrap(); } - writeln!(out); + writeln!(out).unwrap(); } PrintKind::StackProtectorStrategies => { writeln!( @@ -332,7 +333,8 @@ impl CodegenBackend for LlvmCodegenBackend { none Do not generate stack canaries. "# - ); + ) + .unwrap(); } _other => llvm_util::print(req, out, sess), } diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index 7e0f264a4aedf..0e89e66be49a0 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -6,7 +6,6 @@ use crate::errors::{ use crate::llvm; use libc::c_int; use rustc_codegen_ssa::base::wants_wasm_eh; -use rustc_codegen_ssa::traits::PrintBackendInfo; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::small_c_str::SmallCStr; use rustc_fs_util::path_to_c_string; @@ -18,6 +17,7 @@ use rustc_target::spec::{MergeFunctions, PanicStrategy}; use rustc_target::target_features::RUSTC_SPECIFIC_FEATURES; use std::ffi::{c_char, c_void, CStr, CString}; +use std::fmt::Write; use std::path::Path; use std::ptr; use std::slice; @@ -372,7 +372,7 @@ fn llvm_target_features(tm: &llvm::TargetMachine) -> Vec<(&str, &str)> { ret } -fn print_target_features(out: &mut dyn PrintBackendInfo, sess: &Session, tm: &llvm::TargetMachine) { +fn print_target_features(out: &mut String, sess: &Session, tm: &llvm::TargetMachine) { let mut llvm_target_features = llvm_target_features(tm); let mut known_llvm_target_features = FxHashSet::<&'static str>::default(); let mut rustc_target_features = sess @@ -412,24 +412,26 @@ fn print_target_features(out: &mut dyn PrintBackendInfo, sess: &Session, tm: &ll .max() .unwrap_or(0); - writeln!(out, "Features supported by rustc for this target:"); + writeln!(out, "Features supported by rustc for this target:").unwrap(); for (feature, desc) in &rustc_target_features { - writeln!(out, " {feature:max_feature_len$} - {desc}."); + writeln!(out, " {feature:max_feature_len$} - {desc}.").unwrap(); } - writeln!(out, "\nCode-generation features supported by LLVM for this target:"); + writeln!(out, "\nCode-generation features supported by LLVM for this target:").unwrap(); for (feature, desc) in &llvm_target_features { - writeln!(out, " {feature:max_feature_len$} - {desc}."); + writeln!(out, " {feature:max_feature_len$} - {desc}.").unwrap(); } if llvm_target_features.is_empty() { - writeln!(out, " Target features listing is not supported by this LLVM version."); + writeln!(out, " Target features listing is not supported by this LLVM version.") + .unwrap(); } - writeln!(out, "\nUse +feature to enable a feature, or -feature to disable it."); - writeln!(out, "For example, rustc -C target-cpu=mycpu -C target-feature=+feature1,-feature2\n"); - writeln!(out, "Code-generation features cannot be used in cfg or #[target_feature],"); - writeln!(out, "and may be renamed or removed in a future version of LLVM or rustc.\n"); + writeln!(out, "\nUse +feature to enable a feature, or -feature to disable it.").unwrap(); + writeln!(out, "For example, rustc -C target-cpu=mycpu -C target-feature=+feature1,-feature2\n") + .unwrap(); + writeln!(out, "Code-generation features cannot be used in cfg or #[target_feature],").unwrap(); + writeln!(out, "and may be renamed or removed in a future version of LLVM or rustc.\n").unwrap(); } -pub(crate) fn print(req: &PrintRequest, mut out: &mut dyn PrintBackendInfo, sess: &Session) { +pub(crate) fn print(req: &PrintRequest, mut out: &mut String, sess: &Session) { require_inited(); let tm = create_informational_target_machine(sess); match req.kind { @@ -440,9 +442,9 @@ pub(crate) fn print(req: &PrintRequest, mut out: &mut dyn PrintBackendInfo, sess let cpu_cstring = CString::new(handle_native(sess.target.cpu.as_ref())) .unwrap_or_else(|e| bug!("failed to convert to cstring: {}", e)); unsafe extern "C" fn callback(out: *mut c_void, string: *const c_char, len: usize) { - let out = &mut *(out as *mut &mut dyn PrintBackendInfo); + let out = &mut *(out as *mut &mut String); let bytes = slice::from_raw_parts(string as *const u8, len); - write!(out, "{}", String::from_utf8_lossy(bytes)); + write!(out, "{}", String::from_utf8_lossy(bytes)).unwrap(); } unsafe { llvm::LLVMRustPrintTargetCPUs( diff --git a/compiler/rustc_codegen_ssa/src/traits/backend.rs b/compiler/rustc_codegen_ssa/src/traits/backend.rs index e8b9490d4010a..3770bd11cf9b2 100644 --- a/compiler/rustc_codegen_ssa/src/traits/backend.rs +++ b/compiler/rustc_codegen_ssa/src/traits/backend.rs @@ -22,8 +22,6 @@ use rustc_session::{ use rustc_span::symbol::Symbol; use rustc_target::abi::call::FnAbi; -use std::fmt; - pub trait BackendTypes { type Value: CodegenObject; type Function: CodegenObject; @@ -62,7 +60,7 @@ pub trait CodegenBackend { fn locale_resource(&self) -> &'static str; fn init(&self, _sess: &Session) {} - fn print(&self, _req: &PrintRequest, _out: &mut dyn PrintBackendInfo, _sess: &Session) {} + fn print(&self, _req: &PrintRequest, _out: &mut String, _sess: &Session) {} fn target_features(&self, _sess: &Session, _allow_unstable: bool) -> Vec { vec![] } @@ -150,19 +148,3 @@ pub trait ExtraBackendMethods: std::thread::Builder::new().name(name).spawn(f) } } - -pub trait PrintBackendInfo { - fn infallible_write_fmt(&mut self, args: fmt::Arguments<'_>); -} - -impl PrintBackendInfo for String { - fn infallible_write_fmt(&mut self, args: fmt::Arguments<'_>) { - fmt::Write::write_fmt(self, args).unwrap(); - } -} - -impl dyn PrintBackendInfo + '_ { - pub fn write_fmt(&mut self, args: fmt::Arguments<'_>) { - self.infallible_write_fmt(args); - } -} diff --git a/compiler/rustc_codegen_ssa/src/traits/mod.rs b/compiler/rustc_codegen_ssa/src/traits/mod.rs index 728c2bc8c49bc..8cb58bd4c704d 100644 --- a/compiler/rustc_codegen_ssa/src/traits/mod.rs +++ b/compiler/rustc_codegen_ssa/src/traits/mod.rs @@ -30,9 +30,7 @@ mod write; pub use self::abi::AbiBuilderMethods; pub use self::asm::{AsmBuilderMethods, AsmMethods, GlobalAsmOperandRef, InlineAsmOperandRef}; -pub use self::backend::{ - Backend, BackendTypes, CodegenBackend, ExtraBackendMethods, PrintBackendInfo, -}; +pub use self::backend::{Backend, BackendTypes, CodegenBackend, ExtraBackendMethods}; pub use self::builder::{BuilderMethods, OverflowOp}; pub use self::consts::ConstMethods; pub use self::coverageinfo::CoverageInfoBuilderMethods; From 98e8601ac3426c395c017f110e2ff8b05733a163 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 30 Mar 2024 13:32:05 +0000 Subject: [PATCH 03/19] Remove const_bitcast from ConstMethods --- compiler/rustc_codegen_gcc/src/common.rs | 26 +++++++++---------- compiler/rustc_codegen_llvm/src/common.rs | 4 --- .../rustc_codegen_ssa/src/traits/consts.rs | 1 - 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/common.rs b/compiler/rustc_codegen_gcc/src/common.rs index 548c23cc7948a..230fe4f5871e8 100644 --- a/compiler/rustc_codegen_gcc/src/common.rs +++ b/compiler/rustc_codegen_gcc/src/common.rs @@ -28,6 +28,19 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { global // TODO(antoyo): set linkage. } + + pub fn const_bitcast(&self, value: RValue<'gcc>, typ: Type<'gcc>) -> RValue<'gcc> { + if value.get_type() == self.bool_type.make_pointer() { + if let Some(pointee) = typ.get_pointee() { + if pointee.dyncast_vector().is_some() { + panic!() + } + } + } + // NOTE: since bitcast makes a value non-constant, don't bitcast if not necessary as some + // SIMD builtins require a constant value. + self.bitcast_if_needed(value, typ) + } } pub fn bytes_in_context<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, bytes: &[u8]) -> RValue<'gcc> { @@ -239,19 +252,6 @@ impl<'gcc, 'tcx> ConstMethods<'tcx> for CodegenCx<'gcc, 'tcx> { const_alloc_to_gcc(self, alloc) } - fn const_bitcast(&self, value: RValue<'gcc>, typ: Type<'gcc>) -> RValue<'gcc> { - if value.get_type() == self.bool_type.make_pointer() { - if let Some(pointee) = typ.get_pointee() { - if pointee.dyncast_vector().is_some() { - panic!() - } - } - } - // NOTE: since bitcast makes a value non-constant, don't bitcast if not necessary as some - // SIMD builtins require a constant value. - self.bitcast_if_needed(value, typ) - } - fn const_ptr_byte_offset(&self, base_addr: Self::Value, offset: abi::Size) -> Self::Value { self.context .new_array_access(None, base_addr, self.const_usize(offset.bytes())) diff --git a/compiler/rustc_codegen_llvm/src/common.rs b/compiler/rustc_codegen_llvm/src/common.rs index 4ffc92eb63356..d42c6ed827aec 100644 --- a/compiler/rustc_codegen_llvm/src/common.rs +++ b/compiler/rustc_codegen_llvm/src/common.rs @@ -329,10 +329,6 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> { const_alloc_to_llvm(self, alloc, /*static*/ false) } - fn const_bitcast(&self, val: &'ll Value, ty: &'ll Type) -> &'ll Value { - self.const_bitcast(val, ty) - } - fn const_ptr_byte_offset(&self, base_addr: Self::Value, offset: abi::Size) -> Self::Value { unsafe { llvm::LLVMConstInBoundsGEP2( diff --git a/compiler/rustc_codegen_ssa/src/traits/consts.rs b/compiler/rustc_codegen_ssa/src/traits/consts.rs index 8cb17a5b37a89..3da732602c520 100644 --- a/compiler/rustc_codegen_ssa/src/traits/consts.rs +++ b/compiler/rustc_codegen_ssa/src/traits/consts.rs @@ -37,6 +37,5 @@ pub trait ConstMethods<'tcx>: BackendTypes { fn scalar_to_backend(&self, cv: Scalar, layout: abi::Scalar, llty: Self::Type) -> Self::Value; - fn const_bitcast(&self, val: Self::Value, ty: Self::Type) -> Self::Value; fn const_ptr_byte_offset(&self, val: Self::Value, offset: abi::Size) -> Self::Value; } From e32eb4c9e933d98455205348706620c51f25b69d Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 30 Mar 2024 13:53:52 +0000 Subject: [PATCH 04/19] Dedup some intrinsic handling code for caller_location --- compiler/rustc_codegen_ssa/src/mir/block.rs | 37 +++++++-------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 57138d3b9dbdb..57a5393da251e 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -902,31 +902,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // The arguments we'll be passing. Plus one to account for outptr, if used. let arg_count = fn_abi.args.len() + fn_abi.ret.is_indirect() as usize; - if matches!(intrinsic, Some(ty::IntrinsicDef { name: sym::caller_location, .. })) { - return if let Some(target) = target { - let location = - self.get_caller_location(bx, mir::SourceInfo { span: fn_span, ..source_info }); - - let mut llargs = Vec::with_capacity(arg_count); - let ret_dest = self.make_return_dest( - bx, - destination, - &fn_abi.ret, - &mut llargs, - intrinsic, - Some(target), - ); - assert_eq!(llargs, []); - if let ReturnDest::IndirectOperand(tmp, _) = ret_dest { - location.val.store(bx, tmp); - } - self.store_return(bx, ret_dest, &fn_abi.ret, location.immediate()); - helper.funclet_br(self, bx, target, mergeable_succ) - } else { - MergingSucc::False - }; - } - let instance = match intrinsic { None => instance, Some(intrinsic) => { @@ -971,6 +946,18 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { }) .collect(); + if matches!(intrinsic, ty::IntrinsicDef { name: sym::caller_location, .. }) { + let location = self + .get_caller_location(bx, mir::SourceInfo { span: fn_span, ..source_info }); + + assert_eq!(llargs, []); + if let ReturnDest::IndirectOperand(tmp, _) = ret_dest { + location.val.store(bx, tmp); + } + self.store_return(bx, ret_dest, &fn_abi.ret, location.immediate()); + return helper.funclet_br(self, bx, target.unwrap(), mergeable_succ); + } + let instance = *instance.as_ref().unwrap(); match Self::codegen_intrinsic_call(bx, instance, fn_abi, &args, dest, span) { Ok(()) => { From 22b32432ca3e8296a7649ef8c3711c0310f8f426 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 30 Mar 2024 13:58:06 +0000 Subject: [PATCH 05/19] Move all intrinsic handling code in codegen_call_terminators together --- compiler/rustc_codegen_ssa/src/mir/block.rs | 44 +++++++++------------ 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 57a5393da251e..1eec1841a3753 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -751,7 +751,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { &mut self, helper: &TerminatorCodegenHelper<'tcx>, bx: &mut Bx, - intrinsic: Option, + intrinsic: ty::IntrinsicDef, instance: Option>, source_info: mir::SourceInfo, target: Option, @@ -761,8 +761,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // Emit a panic or a no-op for `assert_*` intrinsics. // These are intrinsics that compile to panics so that we can get a message // which mentions the offending type, even from a const context. - let panic_intrinsic = intrinsic.and_then(|i| ValidityRequirement::from_intrinsic(i.name)); - if let Some(requirement) = panic_intrinsic { + if let Some(requirement) = ValidityRequirement::from_intrinsic(intrinsic.name) { let ty = instance.unwrap().args.type_at(0); let do_panic = !bx @@ -869,12 +868,6 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let sig = callee.layout.ty.fn_sig(bx.tcx()); let abi = sig.abi(); - // Handle intrinsics old codegen wants Expr's for, ourselves. - let intrinsic = match def { - Some(ty::InstanceKind::Intrinsic(def_id)) => Some(bx.tcx().intrinsic(def_id).unwrap()), - _ => None, - }; - let extra_args = &args[sig.inputs().skip_binder().len()..]; let extra_args = bx.tcx().mk_type_list_from_iter(extra_args.iter().map(|op_arg| { let op_ty = op_arg.node.ty(self.mir, bx.tcx()); @@ -886,25 +879,25 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { None => bx.fn_abi_of_fn_ptr(sig, extra_args), }; - if let Some(merging_succ) = self.codegen_panic_intrinsic( - &helper, - bx, - intrinsic, - instance, - source_info, - target, - unwind, - mergeable_succ, - ) { - return merging_succ; - } - // The arguments we'll be passing. Plus one to account for outptr, if used. let arg_count = fn_abi.args.len() + fn_abi.ret.is_indirect() as usize; - let instance = match intrinsic { - None => instance, - Some(intrinsic) => { + let instance = match def { + Some(ty::InstanceKind::Intrinsic(def_id)) => { + let intrinsic = bx.tcx().intrinsic(def_id).unwrap(); + if let Some(merging_succ) = self.codegen_panic_intrinsic( + &helper, + bx, + intrinsic, + instance, + source_info, + target, + unwind, + mergeable_succ, + ) { + return merging_succ; + } + let mut llargs = Vec::with_capacity(1); let ret_dest = self.make_return_dest( bx, @@ -984,6 +977,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } } } + _ => instance, }; let mut llargs = Vec::with_capacity(arg_count); From aacdce38f7f2298c5779446ffe17cae0068ccd37 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 30 Mar 2024 14:25:13 +0000 Subject: [PATCH 06/19] Remove check_overflow method from MiscMethods It can be retrieved from the Session too. --- compiler/rustc_codegen_gcc/src/context.rs | 8 -------- compiler/rustc_codegen_llvm/src/context.rs | 8 -------- compiler/rustc_codegen_ssa/src/mir/block.rs | 2 +- compiler/rustc_codegen_ssa/src/traits/misc.rs | 1 - 4 files changed, 1 insertion(+), 18 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/context.rs b/compiler/rustc_codegen_gcc/src/context.rs index 4a1f5188a8013..6231b09552cea 100644 --- a/compiler/rustc_codegen_gcc/src/context.rs +++ b/compiler/rustc_codegen_gcc/src/context.rs @@ -27,7 +27,6 @@ use crate::callee::get_fn; use crate::common::SignType; pub struct CodegenCx<'gcc, 'tcx> { - pub check_overflow: bool, pub codegen_unit: &'tcx CodegenUnit<'tcx>, pub context: &'gcc Context<'gcc>, @@ -134,8 +133,6 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { tcx: TyCtxt<'tcx>, supports_128bit_integers: bool, ) -> Self { - let check_overflow = tcx.sess.overflow_checks(); - let create_type = |ctype, rust_type| { let layout = tcx.layout_of(ParamEnv::reveal_all().and(rust_type)).unwrap(); let align = layout.align.abi.bytes(); @@ -271,7 +268,6 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { } let mut cx = Self { - check_overflow, codegen_unit, context, current_func: RefCell::new(None), @@ -511,10 +507,6 @@ impl<'gcc, 'tcx> MiscMethods<'tcx> for CodegenCx<'gcc, 'tcx> { &self.tcx.sess } - fn check_overflow(&self) -> bool { - self.check_overflow - } - fn codegen_unit(&self) -> &'tcx CodegenUnit<'tcx> { self.codegen_unit } diff --git a/compiler/rustc_codegen_llvm/src/context.rs b/compiler/rustc_codegen_llvm/src/context.rs index 57682d1cc8c4c..1a8e8efdae507 100644 --- a/compiler/rustc_codegen_llvm/src/context.rs +++ b/compiler/rustc_codegen_llvm/src/context.rs @@ -44,7 +44,6 @@ use std::str; /// All other LLVM data structures in the `CodegenCx` are tied to that `llvm::Context`. pub struct CodegenCx<'ll, 'tcx> { pub tcx: TyCtxt<'tcx>, - pub check_overflow: bool, pub use_dll_storage_attrs: bool, pub tls_model: llvm::ThreadLocalMode, @@ -442,8 +441,6 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { // start) and then strongly recommending static linkage on Windows! let use_dll_storage_attrs = tcx.sess.target.is_like_windows; - let check_overflow = tcx.sess.overflow_checks(); - let tls_model = to_llvm_tls_model(tcx.sess.tls_model()); let (llcx, llmod) = (&*llvm_module.llcx, llvm_module.llmod()); @@ -467,7 +464,6 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> { CodegenCx { tcx, - check_overflow, use_dll_storage_attrs, tls_model, llmod, @@ -606,10 +602,6 @@ impl<'ll, 'tcx> MiscMethods<'tcx> for CodegenCx<'ll, 'tcx> { self.tcx.sess } - fn check_overflow(&self) -> bool { - self.check_overflow - } - fn codegen_unit(&self) -> &'tcx CodegenUnit<'tcx> { self.codegen_unit } diff --git a/compiler/rustc_codegen_ssa/src/mir/block.rs b/compiler/rustc_codegen_ssa/src/mir/block.rs index 1eec1841a3753..b1c22faf1ae9d 100644 --- a/compiler/rustc_codegen_ssa/src/mir/block.rs +++ b/compiler/rustc_codegen_ssa/src/mir/block.rs @@ -658,7 +658,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // with #[rustc_inherit_overflow_checks] and inlined from // another crate (mostly core::num generic/#[inline] fns), // while the current crate doesn't use overflow checks. - if !bx.cx().check_overflow() && msg.is_optional_overflow_check() { + if !bx.sess().overflow_checks() && msg.is_optional_overflow_check() { const_cond = Some(expected); } diff --git a/compiler/rustc_codegen_ssa/src/traits/misc.rs b/compiler/rustc_codegen_ssa/src/traits/misc.rs index af3a998960491..0ace28ed3ba5e 100644 --- a/compiler/rustc_codegen_ssa/src/traits/misc.rs +++ b/compiler/rustc_codegen_ssa/src/traits/misc.rs @@ -16,7 +16,6 @@ pub trait MiscMethods<'tcx>: BackendTypes { _vtable: Self::Value, ) { } - fn check_overflow(&self) -> bool; fn get_fn(&self, instance: Instance<'tcx>) -> Self::Function; fn get_fn_addr(&self, instance: Instance<'tcx>) -> Self::Value; fn eh_personality(&self) -> Self::Value; From 887f57ff0bd0664bfb901321048302f0879e1be2 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 30 Mar 2024 14:36:14 +0000 Subject: [PATCH 07/19] Remove type_i1 and type_struct from cg_ssa They are not representable by Cranelift --- compiler/rustc_codegen_gcc/src/type_.rs | 50 +++++++++---------- compiler/rustc_codegen_llvm/src/type_.rs | 28 +++++------ .../rustc_codegen_ssa/src/traits/type_.rs | 2 - 3 files changed, 39 insertions(+), 41 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/type_.rs b/compiler/rustc_codegen_gcc/src/type_.rs index 68471b028beb8..4caff2e63106a 100644 --- a/compiler/rustc_codegen_gcc/src/type_.rs +++ b/compiler/rustc_codegen_gcc/src/type_.rs @@ -89,13 +89,34 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { ty::FloatTy::F128 => self.type_f128(), } } -} -impl<'gcc, 'tcx> BaseTypeMethods<'tcx> for CodegenCx<'gcc, 'tcx> { - fn type_i1(&self) -> Type<'gcc> { + pub fn type_i1(&self) -> Type<'gcc> { self.bool_type } + pub fn type_struct(&self, fields: &[Type<'gcc>], packed: bool) -> Type<'gcc> { + let types = fields.to_vec(); + if let Some(typ) = self.struct_types.borrow().get(fields) { + return *typ; + } + let fields: Vec<_> = fields + .iter() + .enumerate() + .map(|(index, field)| { + self.context.new_field(None, *field, format!("field{}_TODO", index)) + }) + .collect(); + let typ = self.context.new_struct_type(None, "struct", &fields).as_type(); + if packed { + #[cfg(feature = "master")] + typ.set_packed(); + } + self.struct_types.borrow_mut().insert(types, typ); + typ + } +} + +impl<'gcc, 'tcx> BaseTypeMethods<'tcx> for CodegenCx<'gcc, 'tcx> { fn type_i8(&self) -> Type<'gcc> { self.i8_type } @@ -131,7 +152,7 @@ impl<'gcc, 'tcx> BaseTypeMethods<'tcx> for CodegenCx<'gcc, 'tcx> { fn type_f64(&self) -> Type<'gcc> { self.double_type } - + fn type_f128(&self) -> Type<'gcc> { unimplemented!("f16_f128") } @@ -140,27 +161,6 @@ impl<'gcc, 'tcx> BaseTypeMethods<'tcx> for CodegenCx<'gcc, 'tcx> { self.context.new_function_pointer_type(None, return_type, params, false) } - fn type_struct(&self, fields: &[Type<'gcc>], packed: bool) -> Type<'gcc> { - let types = fields.to_vec(); - if let Some(typ) = self.struct_types.borrow().get(fields) { - return *typ; - } - let fields: Vec<_> = fields - .iter() - .enumerate() - .map(|(index, field)| { - self.context.new_field(None, *field, format!("field{}_TODO", index)) - }) - .collect(); - let typ = self.context.new_struct_type(None, "struct", &fields).as_type(); - if packed { - #[cfg(feature = "master")] - typ.set_packed(); - } - self.struct_types.borrow_mut().insert(types, typ); - typ - } - fn type_kind(&self, typ: Type<'gcc>) -> TypeKind { if self.is_int_type_or_bool(typ) { TypeKind::Integer diff --git a/compiler/rustc_codegen_llvm/src/type_.rs b/compiler/rustc_codegen_llvm/src/type_.rs index a00f09dc40da6..f1141c57cedd7 100644 --- a/compiler/rustc_codegen_llvm/src/type_.rs +++ b/compiler/rustc_codegen_llvm/src/type_.rs @@ -127,13 +127,24 @@ impl<'ll> CodegenCx<'ll, '_> { pub(crate) fn type_variadic_func(&self, args: &[&'ll Type], ret: &'ll Type) -> &'ll Type { unsafe { llvm::LLVMFunctionType(ret, args.as_ptr(), args.len() as c_uint, True) } } -} -impl<'ll, 'tcx> BaseTypeMethods<'tcx> for CodegenCx<'ll, 'tcx> { - fn type_i1(&self) -> &'ll Type { + pub(crate) fn type_i1(&self) -> &'ll Type { unsafe { llvm::LLVMInt1TypeInContext(self.llcx) } } + pub(crate) fn type_struct(&self, els: &[&'ll Type], packed: bool) -> &'ll Type { + unsafe { + llvm::LLVMStructTypeInContext( + self.llcx, + els.as_ptr(), + els.len() as c_uint, + packed as Bool, + ) + } + } +} + +impl<'ll, 'tcx> BaseTypeMethods<'tcx> for CodegenCx<'ll, 'tcx> { fn type_i8(&self) -> &'ll Type { unsafe { llvm::LLVMInt8TypeInContext(self.llcx) } } @@ -178,17 +189,6 @@ impl<'ll, 'tcx> BaseTypeMethods<'tcx> for CodegenCx<'ll, 'tcx> { unsafe { llvm::LLVMFunctionType(ret, args.as_ptr(), args.len() as c_uint, False) } } - fn type_struct(&self, els: &[&'ll Type], packed: bool) -> &'ll Type { - unsafe { - llvm::LLVMStructTypeInContext( - self.llcx, - els.as_ptr(), - els.len() as c_uint, - packed as Bool, - ) - } - } - fn type_kind(&self, ty: &'ll Type) -> TypeKind { unsafe { llvm::LLVMRustGetTypeKind(ty).to_generic() } } diff --git a/compiler/rustc_codegen_ssa/src/traits/type_.rs b/compiler/rustc_codegen_ssa/src/traits/type_.rs index 403f6a7327715..e492bba969e1a 100644 --- a/compiler/rustc_codegen_ssa/src/traits/type_.rs +++ b/compiler/rustc_codegen_ssa/src/traits/type_.rs @@ -12,7 +12,6 @@ use rustc_target::abi::{AddressSpace, Float, Integer}; // This depends on `Backend` and not `BackendTypes`, because consumers will probably want to use // `LayoutOf` or `HasTyCtxt`. This way, they don't have to add a constraint on it themselves. pub trait BaseTypeMethods<'tcx>: Backend<'tcx> { - fn type_i1(&self) -> Self::Type; fn type_i8(&self) -> Self::Type; fn type_i16(&self) -> Self::Type; fn type_i32(&self) -> Self::Type; @@ -27,7 +26,6 @@ pub trait BaseTypeMethods<'tcx>: Backend<'tcx> { fn type_array(&self, ty: Self::Type, len: u64) -> Self::Type; fn type_func(&self, args: &[Self::Type], ret: Self::Type) -> Self::Type; - fn type_struct(&self, els: &[Self::Type], packed: bool) -> Self::Type; fn type_kind(&self, ty: Self::Type) -> TypeKind; fn type_ptr(&self) -> Self::Type; fn type_ptr_ext(&self, address_space: AddressSpace) -> Self::Type; From 84f45bb0930a0696f8d0bcb8b480bc194da80db1 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sat, 30 Mar 2024 17:29:32 +0000 Subject: [PATCH 08/19] Fix doc comment --- compiler/rustc_codegen_ssa/src/traits/type_.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/traits/type_.rs b/compiler/rustc_codegen_ssa/src/traits/type_.rs index e492bba969e1a..b1bad6cfa6f58 100644 --- a/compiler/rustc_codegen_ssa/src/traits/type_.rs +++ b/compiler/rustc_codegen_ssa/src/traits/type_.rs @@ -113,8 +113,8 @@ pub trait LayoutTypeMethods<'tcx>: Backend<'tcx> { /// The backend type used for a rust type when it's in an SSA register. /// /// For nearly all types this is the same as the [`Self::backend_type`], however - /// `bool` (and other `0`-or-`1` values) are kept as [`BaseTypeMethods::type_i1`] - /// in registers but as [`BaseTypeMethods::type_i8`] in memory. + /// `bool` (and other `0`-or-`1` values) are kept as `i1` in registers but as + /// [`BaseTypeMethods::type_i8`] in memory. /// /// Converting values between the two different backend types is done using /// [`from_immediate`](super::BuilderMethods::from_immediate) and From 8a87f02138ad4dae9a57d71635f9be2bf5696202 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Tue, 25 Jun 2024 20:46:13 +0200 Subject: [PATCH 09/19] Improve error message in tidy --- src/tools/tidy/src/ext_tool_checks.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/tools/tidy/src/ext_tool_checks.rs b/src/tools/tidy/src/ext_tool_checks.rs index 995ad58cb6219..26af51bd2d744 100644 --- a/src/tools/tidy/src/ext_tool_checks.rs +++ b/src/tools/tidy/src/ext_tool_checks.rs @@ -267,11 +267,10 @@ fn create_venv_at_path(path: &Path) -> Result<(), Error> { let stderr = String::from_utf8_lossy(&out.stderr); let err = if stderr.contains("No module named virtualenv") { - Error::Generic( + Error::Generic(format!( "virtualenv not found: you may need to install it \ - (`python3 -m pip install venv`)" - .to_owned(), - ) + (`{sys_py} -m pip install virtualenv`)" + )) } else { Error::Generic(format!( "failed to create venv at '{}' using {sys_py}: {stderr}", From 151986f493bce146db808db2bbb2b29fa6e0049c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 27 Jun 2024 10:22:03 +0200 Subject: [PATCH 10/19] Implement `x perf` as a separate tool --- Cargo.lock | 7 ++ Cargo.toml | 1 + src/bootstrap/src/core/build_steps/perf.rs | 32 ++--- src/bootstrap/src/core/build_steps/tool.rs | 1 + src/bootstrap/src/core/config/flags.rs | 4 +- src/etc/completions/x.py.fish | 2 +- src/etc/completions/x.py.ps1 | 2 +- src/etc/completions/x.py.zsh | 2 +- src/tools/rustc-perf-wrapper/Cargo.toml | 7 ++ src/tools/rustc-perf-wrapper/README.md | 3 + src/tools/rustc-perf-wrapper/src/config.rs | 45 +++++++ src/tools/rustc-perf-wrapper/src/main.rs | 130 +++++++++++++++++++++ 12 files changed, 211 insertions(+), 25 deletions(-) create mode 100644 src/tools/rustc-perf-wrapper/Cargo.toml create mode 100644 src/tools/rustc-perf-wrapper/README.md create mode 100644 src/tools/rustc-perf-wrapper/src/config.rs create mode 100644 src/tools/rustc-perf-wrapper/src/main.rs diff --git a/Cargo.lock b/Cargo.lock index 241a37588b409..b1b7020f29000 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3453,6 +3453,13 @@ dependencies = [ "stable_mir", ] +[[package]] +name = "rustc-perf-wrapper" +version = "0.1.0" +dependencies = [ + "clap", +] + [[package]] name = "rustc-rayon" version = "0.5.0" diff --git a/Cargo.toml b/Cargo.toml index c17ea99d03767..93c520b0d689d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,6 +44,7 @@ members = [ "src/tools/rustdoc-gui-test", "src/tools/opt-dist", "src/tools/coverage-dump", + "src/tools/rustc-perf-wrapper", ] exclude = [ diff --git a/src/bootstrap/src/core/build_steps/perf.rs b/src/bootstrap/src/core/build_steps/perf.rs index 9d70ca6bd71fd..f8170580722bf 100644 --- a/src/bootstrap/src/core/build_steps/perf.rs +++ b/src/bootstrap/src/core/build_steps/perf.rs @@ -1,7 +1,5 @@ -use std::process::Command; - use crate::core::build_steps::compile::{Std, Sysroot}; -use crate::core::build_steps::tool::RustcPerf; +use crate::core::build_steps::tool::{RustcPerf, Tool}; use crate::core::builder::Builder; use crate::core::config::DebuginfoLevel; @@ -22,24 +20,16 @@ Consider setting `rust.debuginfo-level = 1` in `config.toml`."#); let sysroot = builder.ensure(Sysroot::new(compiler)); let rustc = sysroot.join("bin/rustc"); - let results_dir = builder.build.tempdir().join("rustc-perf"); - - let mut cmd = Command::new(collector); - let cmd = cmd - .arg("profile_local") - .arg("eprintln") - .arg("--out-dir") - .arg(&results_dir) - .arg("--include") - .arg("helloworld") - .arg(&rustc); - - builder.info(&format!("Running `rustc-perf` using `{}`", rustc.display())); + let rustc_perf_dir = builder.build.tempdir().join("rustc-perf"); + let profile_results_dir = rustc_perf_dir.join("results"); - // We need to set the working directory to `src/tools/perf`, so that it can find the directory - // with compile-time benchmarks. - let cmd = cmd.current_dir(builder.src.join("src/tools/rustc-perf")); - builder.build.run(cmd); + // We need to take args passed after `--` and pass them to `rustc-perf-wrapper` + let args = std::env::args().skip_while(|a| a != "--").skip(1); - builder.info(&format!("You can find the results at `{}`", results_dir.display())); + let mut cmd = builder.tool_cmd(Tool::RustcPerfWrapper); + cmd.env("PERF_RUSTC", rustc) + .env("PERF_COLLECTOR", collector) + .env("PERF_RESULT_DIR", profile_results_dir) + .args(args); + builder.run(&mut cmd); } diff --git a/src/bootstrap/src/core/build_steps/tool.rs b/src/bootstrap/src/core/build_steps/tool.rs index 7411d0ba2befe..2ceca7305a621 100644 --- a/src/bootstrap/src/core/build_steps/tool.rs +++ b/src/bootstrap/src/core/build_steps/tool.rs @@ -336,6 +336,7 @@ bootstrap_tool!( GenerateWindowsSys, "src/tools/generate-windows-sys", "generate-windows-sys"; RustdocGUITest, "src/tools/rustdoc-gui-test", "rustdoc-gui-test", is_unstable_tool = true, allow_features = "test"; CoverageDump, "src/tools/coverage-dump", "coverage-dump"; + RustcPerfWrapper, "src/tools/rustc-perf-wrapper", "rustc-perf-wrapper"; ); #[derive(Debug, Clone, Hash, PartialEq, Eq)] diff --git a/src/bootstrap/src/core/config/flags.rs b/src/bootstrap/src/core/config/flags.rs index eb5152a38312b..aeb608a9ea26b 100644 --- a/src/bootstrap/src/core/config/flags.rs +++ b/src/bootstrap/src/core/config/flags.rs @@ -470,7 +470,9 @@ Arguments: versioned_dirs: bool, }, /// Perform profiling and benchmarking of the compiler using the - /// `rustc-perf` benchmark suite. + /// `rustc-perf-wrapper` tool. + /// + /// You need to pass arguments after `--`, e.g.`x perf -- cachegrind`. Perf {}, } diff --git a/src/etc/completions/x.py.fish b/src/etc/completions/x.py.fish index 2072f76a48181..805fc8aa8ccd0 100644 --- a/src/etc/completions/x.py.fish +++ b/src/etc/completions/x.py.fish @@ -48,7 +48,7 @@ complete -c x.py -n "__fish_use_subcommand" -f -a "run" -d 'Run tools contained complete -c x.py -n "__fish_use_subcommand" -f -a "setup" -d 'Set up the environment for development' complete -c x.py -n "__fish_use_subcommand" -f -a "suggest" -d 'Suggest a subset of tests to run, based on modified files' complete -c x.py -n "__fish_use_subcommand" -f -a "vendor" -d 'Vendor dependencies' -complete -c x.py -n "__fish_use_subcommand" -f -a "perf" -d 'Perform profiling and benchmarking of the compiler using the `rustc-perf` benchmark suite' +complete -c x.py -n "__fish_use_subcommand" -f -a "perf" -d 'Perform profiling and benchmarking of the compiler using the `rustc-perf-wrapper` tool' complete -c x.py -n "__fish_seen_subcommand_from build" -l config -d 'TOML configuration file for build' -r -F complete -c x.py -n "__fish_seen_subcommand_from build" -l build-dir -d 'Build directory, overrides `build.build-dir` in `config.toml`' -r -f -a "(__fish_complete_directories)" complete -c x.py -n "__fish_seen_subcommand_from build" -l build -d 'build target of the stage0 compiler' -r -f diff --git a/src/etc/completions/x.py.ps1 b/src/etc/completions/x.py.ps1 index 919382d441ffc..ce590d2fa4897 100644 --- a/src/etc/completions/x.py.ps1 +++ b/src/etc/completions/x.py.ps1 @@ -75,7 +75,7 @@ Register-ArgumentCompleter -Native -CommandName 'x.py' -ScriptBlock { [CompletionResult]::new('setup', 'setup', [CompletionResultType]::ParameterValue, 'Set up the environment for development') [CompletionResult]::new('suggest', 'suggest', [CompletionResultType]::ParameterValue, 'Suggest a subset of tests to run, based on modified files') [CompletionResult]::new('vendor', 'vendor', [CompletionResultType]::ParameterValue, 'Vendor dependencies') - [CompletionResult]::new('perf', 'perf', [CompletionResultType]::ParameterValue, 'Perform profiling and benchmarking of the compiler using the `rustc-perf` benchmark suite') + [CompletionResult]::new('perf', 'perf', [CompletionResultType]::ParameterValue, 'Perform profiling and benchmarking of the compiler using the `rustc-perf-wrapper` tool') break } 'x.py;build' { diff --git a/src/etc/completions/x.py.zsh b/src/etc/completions/x.py.zsh index bbebf8b892d1f..fc8be4f788127 100644 --- a/src/etc/completions/x.py.zsh +++ b/src/etc/completions/x.py.zsh @@ -856,7 +856,7 @@ _x.py_commands() { 'setup:Set up the environment for development' \ 'suggest:Suggest a subset of tests to run, based on modified files' \ 'vendor:Vendor dependencies' \ -'perf:Perform profiling and benchmarking of the compiler using the \`rustc-perf\` benchmark suite' \ +'perf:Perform profiling and benchmarking of the compiler using the \`rustc-perf-wrapper\` tool' \ ) _describe -t commands 'x.py commands' commands "$@" } diff --git a/src/tools/rustc-perf-wrapper/Cargo.toml b/src/tools/rustc-perf-wrapper/Cargo.toml new file mode 100644 index 0000000000000..416bfef41d7fd --- /dev/null +++ b/src/tools/rustc-perf-wrapper/Cargo.toml @@ -0,0 +1,7 @@ +[package] +name = "rustc-perf-wrapper" +version = "0.1.0" +edition = "2021" + +[dependencies] +clap = { version = "4.5.7", features = ["derive", "env"] } diff --git a/src/tools/rustc-perf-wrapper/README.md b/src/tools/rustc-perf-wrapper/README.md new file mode 100644 index 0000000000000..7c096e3081416 --- /dev/null +++ b/src/tools/rustc-perf-wrapper/README.md @@ -0,0 +1,3 @@ +# rustc-perf wrapper +Utility tool for invoking [`rustc-perf`](https://github.com/rust-lang/rustc-perf) for benchmarking/profiling +a stage1/2 compiler built by bootstrap using `x run perf`. diff --git a/src/tools/rustc-perf-wrapper/src/config.rs b/src/tools/rustc-perf-wrapper/src/config.rs new file mode 100644 index 0000000000000..a88abfe472377 --- /dev/null +++ b/src/tools/rustc-perf-wrapper/src/config.rs @@ -0,0 +1,45 @@ +use std::fmt::{Display, Formatter}; + +#[derive(Clone, Copy, Debug, clap::ValueEnum)] +#[value(rename_all = "PascalCase")] +pub enum Profile { + Check, + Debug, + Doc, + Opt, + Clippy, +} + +impl Display for Profile { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let name = match self { + Profile::Check => "Check", + Profile::Debug => "Debug", + Profile::Doc => "Doc", + Profile::Opt => "Opt", + Profile::Clippy => "Clippy", + }; + f.write_str(name) + } +} + +#[derive(Clone, Copy, Debug, clap::ValueEnum)] +#[value(rename_all = "PascalCase")] +pub enum Scenario { + Full, + IncrFull, + IncrUnchanged, + IncrPatched, +} + +impl Display for Scenario { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let name = match self { + Scenario::Full => "Full", + Scenario::IncrFull => "IncrFull", + Scenario::IncrUnchanged => "IncrUnchanged", + Scenario::IncrPatched => "IncrPatched", + }; + f.write_str(name) + } +} diff --git a/src/tools/rustc-perf-wrapper/src/main.rs b/src/tools/rustc-perf-wrapper/src/main.rs new file mode 100644 index 0000000000000..0974661f99787 --- /dev/null +++ b/src/tools/rustc-perf-wrapper/src/main.rs @@ -0,0 +1,130 @@ +use crate::config::{Profile, Scenario}; +use clap::Parser; +use std::path::PathBuf; +use std::process::Command; + +mod config; + +/// Performs profiling or benchmarking with [`rustc-perf`](https://github.com/rust-lang/rustc-perf) +/// using a locally built compiler. +#[derive(Debug, clap::Parser)] +// Hide arguments from BuildContext in the default usage string. +// Clap does not seem to have a way of disabling the usage of these arguments. +#[clap(override_usage = "rustc-perf-wrapper [OPTIONS] ")] +pub struct Args { + #[clap(subcommand)] + cmd: PerfCommand, + + #[clap(flatten)] + opts: SharedOpts, + + #[clap(flatten)] + ctx: BuildContext, +} + +#[derive(Debug, clap::Parser)] +enum PerfCommand { + /// Run `profile_local eprintln`. + /// This executes the compiler on the given benchmarks and stores its stderr output. + Eprintln, + /// Run `profile_local samply` + /// This executes the compiler on the given benchmarks and profiles it with `samply`. + /// You need to install `samply`, e.g. using `cargo install samply`. + Samply, + /// Run `profile_local cachegrind`. + /// This executes the compiler on the given benchmarks under `Cachegrind`. + Cachegrind, +} + +impl PerfCommand { + fn is_profiling(&self) -> bool { + match self { + PerfCommand::Eprintln | PerfCommand::Samply | PerfCommand::Cachegrind => true, + } + } +} + +#[derive(Debug, clap::Parser)] +struct SharedOpts { + /// Select the benchmarks that you want to run (separated by commas). + /// If unspecified, all benchmarks will be executed. + #[clap(long, global = true, value_delimiter = ',')] + include: Vec, + /// Select the scenarios that should be benchmarked. + #[clap( + long, + global = true, + value_delimiter = ',', + default_value = "Full,IncrFull,IncrUnchanged,IncrPatched" + )] + scenarios: Vec, + /// Select the profiles that should be benchmarked. + #[clap(long, global = true, value_delimiter = ',', default_value = "Check,Debug,Opt")] + profiles: Vec, +} + +/// These arguments are mostly designed to be passed from bootstrap, not by users +/// directly. +#[derive(Debug, clap::Parser)] +struct BuildContext { + /// Compiler binary that will be benchmarked/profiled. + #[clap(long, hide = true, env = "PERF_RUSTC")] + compiler: PathBuf, + /// rustc-perf collector binary that will be used for running benchmarks/profilers. + #[clap(long, hide = true, env = "PERF_COLLECTOR")] + collector: PathBuf, + /// Directory where to store results. + #[clap(long, hide = true, env = "PERF_RESULT_DIR")] + results_dir: PathBuf, +} + +fn main() { + let args = Args::parse(); + run(args); +} + +fn run(args: Args) { + let mut cmd = Command::new(args.ctx.collector); + match &args.cmd { + PerfCommand::Eprintln => { + cmd.arg("profile_local").arg("eprintln"); + } + PerfCommand::Samply => { + cmd.arg("profile_local").arg("samply"); + } + PerfCommand::Cachegrind => { + cmd.arg("profile_local").arg("cachegrind"); + } + } + if args.cmd.is_profiling() { + cmd.arg("--out-dir").arg(&args.ctx.results_dir); + } + + if !args.opts.include.is_empty() { + cmd.arg("--include").arg(args.opts.include.join(",")); + } + if !args.opts.profiles.is_empty() { + cmd.arg("--profiles") + .arg(args.opts.profiles.iter().map(|p| p.to_string()).collect::>().join(",")); + } + if !args.opts.scenarios.is_empty() { + cmd.arg("--scenarios") + .arg(args.opts.scenarios.iter().map(|p| p.to_string()).collect::>().join(",")); + } + cmd.arg(&args.ctx.compiler); + + println!("Running `rustc-perf` using `{}`", args.ctx.compiler.display()); + + const MANIFEST_DIR: &str = env!("CARGO_MANIFEST_DIR"); + + let rustc_perf_dir = PathBuf::from(MANIFEST_DIR).join("../rustc-perf"); + + // We need to set the working directory to `src/tools/perf`, so that it can find the directory + // with compile-time benchmarks. + let cmd = cmd.current_dir(rustc_perf_dir); + cmd.status().expect("error while running rustc-perf collector"); + + if args.cmd.is_profiling() { + println!("You can find the results at `{}`", args.ctx.results_dir.display()); + } +} From a62cbda57e23105d68d146cafce94b882882c0e1 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 28 Jun 2024 23:13:33 -0300 Subject: [PATCH 11/19] Add feature diagnostic for unsafe_extern_blocks --- compiler/rustc_ast_passes/src/ast_validation.rs | 10 +++++++++- .../feature-gate-unsafe-extern-blocks.stderr | 4 ++++ tests/ui/parser/unsafe-foreign-mod-2.stderr | 4 ++++ tests/ui/parser/unsafe-foreign-mod.stderr | 4 ++++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index ba4b6130b60c8..d02b8510975fc 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -1088,7 +1088,15 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } } } else if let &Safety::Unsafe(span) = safety { - this.dcx().emit_err(errors::UnsafeItem { span, kind: "extern block" }); + let mut diag = this + .dcx() + .create_err(errors::UnsafeItem { span, kind: "extern block" }); + rustc_session::parse::add_feature_diagnostics( + &mut diag, + self.session, + sym::unsafe_extern_blocks, + ); + diag.emit(); } if abi.is_none() { diff --git a/tests/ui/feature-gates/feature-gate-unsafe-extern-blocks.stderr b/tests/ui/feature-gates/feature-gate-unsafe-extern-blocks.stderr index 84f00827c6010..5653494630899 100644 --- a/tests/ui/feature-gates/feature-gate-unsafe-extern-blocks.stderr +++ b/tests/ui/feature-gates/feature-gate-unsafe-extern-blocks.stderr @@ -3,6 +3,10 @@ error: extern block cannot be declared unsafe | LL | unsafe extern "C" { | ^^^^^^ + | + = note: see issue #123743 for more information + = help: add `#![feature(unsafe_extern_blocks)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error[E0658]: `unsafe extern {}` blocks and `safe` keyword are experimental --> $DIR/feature-gate-unsafe-extern-blocks.rs:9:5 diff --git a/tests/ui/parser/unsafe-foreign-mod-2.stderr b/tests/ui/parser/unsafe-foreign-mod-2.stderr index 77a383d5efa26..07dbd5568d053 100644 --- a/tests/ui/parser/unsafe-foreign-mod-2.stderr +++ b/tests/ui/parser/unsafe-foreign-mod-2.stderr @@ -9,6 +9,10 @@ error: extern block cannot be declared unsafe | LL | extern "C" unsafe { | ^^^^^^ + | + = note: see issue #123743 for more information + = help: add `#![feature(unsafe_extern_blocks)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error: items in unadorned `extern` blocks cannot have safety qualifiers --> $DIR/unsafe-foreign-mod-2.rs:4:5 diff --git a/tests/ui/parser/unsafe-foreign-mod.stderr b/tests/ui/parser/unsafe-foreign-mod.stderr index 77f6e93be10bb..60b918a89b34d 100644 --- a/tests/ui/parser/unsafe-foreign-mod.stderr +++ b/tests/ui/parser/unsafe-foreign-mod.stderr @@ -3,6 +3,10 @@ error: extern block cannot be declared unsafe | LL | unsafe extern "C" { | ^^^^^^ + | + = note: see issue #123743 for more information + = help: add `#![feature(unsafe_extern_blocks)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error: aborting due to 1 previous error From 50edb32939898fa6c22f3e2b2596317a031acaa7 Mon Sep 17 00:00:00 2001 From: surechen Date: Sat, 29 Jun 2024 14:23:33 +0800 Subject: [PATCH 12/19] Fix a error suggestion for E0121 when using placeholder _ as return types on function signature. Recommit after refactoring based on comment: https://github.com/rust-lang/rust/pull/126017#issuecomment-2189149361 But when changing return type's lifetime to `ReError` will affect the subsequent borrow check process and cause test11 in typeck_type_placeholder_item.rs to lost E0515 message. ```rust fn test11(x: &usize) -> &_ { //~^ ERROR the placeholder `_` is not allowed within types on item signatures for return types &x //~ ERROR cannot return reference to function parameter(this E0515 msg will disappear) } ``` --- compiler/rustc_hir_analysis/src/collect.rs | 19 ++++++++- ...er-return-ty-for-fn-sig-issue-125488.fixed | 33 ++++++++++++++++ ...infer-return-ty-for-fn-sig-issue-125488.rs | 33 ++++++++++++++++ ...r-return-ty-for-fn-sig-issue-125488.stderr | 39 +++++++++++++++++++ .../ui/typeck/typeck_type_placeholder_item.rs | 2 +- .../typeck_type_placeholder_item.stderr | 12 ++---- 6 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 tests/ui/return/infer-return-ty-for-fn-sig-issue-125488.fixed create mode 100644 tests/ui/return/infer-return-ty-for-fn-sig-issue-125488.rs create mode 100644 tests/ui/return/infer-return-ty-for-fn-sig-issue-125488.stderr diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index c6e8759327f03..36d0704b5b036 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -1459,8 +1459,25 @@ fn infer_return_ty_for_fn_sig<'tcx>( Some(ty) => { let fn_sig = tcx.typeck(def_id).liberated_fn_sigs()[hir_id]; // Typeck doesn't expect erased regions to be returned from `type_of`. + // This is a heuristic approach. If the scope has region paramters, + // we should change fn_sig's lifetime from `ReErased` to `ReError`, + // otherwise to `ReStatic`. + let has_region_params = generics.params.iter().any(|param| match param.kind { + GenericParamKind::Lifetime { .. } => true, + _ => false, + }); let fn_sig = tcx.fold_regions(fn_sig, |r, _| match *r { - ty::ReErased => tcx.lifetimes.re_static, + ty::ReErased => { + if has_region_params { + ty::Region::new_error_with_message( + tcx, + DUMMY_SP, + "erased region is not allowed here in return type", + ) + } else { + tcx.lifetimes.re_static + } + } _ => r, }); diff --git a/tests/ui/return/infer-return-ty-for-fn-sig-issue-125488.fixed b/tests/ui/return/infer-return-ty-for-fn-sig-issue-125488.fixed new file mode 100644 index 0000000000000..442ade6abf16d --- /dev/null +++ b/tests/ui/return/infer-return-ty-for-fn-sig-issue-125488.fixed @@ -0,0 +1,33 @@ +//@ run-rustfix + +#[allow(dead_code)] + +fn main() { + struct S<'a>(&'a ()); + + fn f1(s: S<'_>) -> S<'_> { + //~^ ERROR the placeholder `_` is not allowed + s + } + + fn f2(s: S<'_>) -> S<'_> { + //~^ ERROR the placeholder `_` is not allowed + let x = true; + if x { + s + } else { + s + } + } + + fn f3(s: S<'_>) -> S<'_> { + //~^ ERROR the placeholder `_` is not allowed + return s; + } + + fn f4(s: S<'_>) -> S<'_> { + //~^ ERROR the placeholder `_` is not allowed + let _x = 1; + return s; + } +} diff --git a/tests/ui/return/infer-return-ty-for-fn-sig-issue-125488.rs b/tests/ui/return/infer-return-ty-for-fn-sig-issue-125488.rs new file mode 100644 index 0000000000000..04ea3a28addfb --- /dev/null +++ b/tests/ui/return/infer-return-ty-for-fn-sig-issue-125488.rs @@ -0,0 +1,33 @@ +//@ run-rustfix + +#[allow(dead_code)] + +fn main() { + struct S<'a>(&'a ()); + + fn f1(s: S<'_>) -> _ { + //~^ ERROR the placeholder `_` is not allowed + s + } + + fn f2(s: S<'_>) -> _ { + //~^ ERROR the placeholder `_` is not allowed + let x = true; + if x { + s + } else { + s + } + } + + fn f3(s: S<'_>) -> _ { + //~^ ERROR the placeholder `_` is not allowed + return s; + } + + fn f4(s: S<'_>) -> _ { + //~^ ERROR the placeholder `_` is not allowed + let _x = 1; + return s; + } +} diff --git a/tests/ui/return/infer-return-ty-for-fn-sig-issue-125488.stderr b/tests/ui/return/infer-return-ty-for-fn-sig-issue-125488.stderr new file mode 100644 index 0000000000000..8b7c5e1681ad1 --- /dev/null +++ b/tests/ui/return/infer-return-ty-for-fn-sig-issue-125488.stderr @@ -0,0 +1,39 @@ +error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types + --> $DIR/infer-return-ty-for-fn-sig-issue-125488.rs:8:24 + | +LL | fn f1(s: S<'_>) -> _ { + | ^ + | | + | not allowed in type signatures + | help: replace with the correct return type: `S<'_>` + +error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types + --> $DIR/infer-return-ty-for-fn-sig-issue-125488.rs:13:24 + | +LL | fn f2(s: S<'_>) -> _ { + | ^ + | | + | not allowed in type signatures + | help: replace with the correct return type: `S<'_>` + +error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types + --> $DIR/infer-return-ty-for-fn-sig-issue-125488.rs:23:24 + | +LL | fn f3(s: S<'_>) -> _ { + | ^ + | | + | not allowed in type signatures + | help: replace with the correct return type: `S<'_>` + +error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types + --> $DIR/infer-return-ty-for-fn-sig-issue-125488.rs:28:24 + | +LL | fn f4(s: S<'_>) -> _ { + | ^ + | | + | not allowed in type signatures + | help: replace with the correct return type: `S<'_>` + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0121`. diff --git a/tests/ui/typeck/typeck_type_placeholder_item.rs b/tests/ui/typeck/typeck_type_placeholder_item.rs index a95b44e807c5d..29a21a1f45f56 100644 --- a/tests/ui/typeck/typeck_type_placeholder_item.rs +++ b/tests/ui/typeck/typeck_type_placeholder_item.rs @@ -47,7 +47,7 @@ impl Test9 { fn test11(x: &usize) -> &_ { //~^ ERROR the placeholder `_` is not allowed within types on item signatures for return types - &x //~ ERROR cannot return reference to function parameter + &x } unsafe fn test12(x: *const usize) -> *const *const _ { diff --git a/tests/ui/typeck/typeck_type_placeholder_item.stderr b/tests/ui/typeck/typeck_type_placeholder_item.stderr index 7977504dae1d6..9d295f88da5aa 100644 --- a/tests/ui/typeck/typeck_type_placeholder_item.stderr +++ b/tests/ui/typeck/typeck_type_placeholder_item.stderr @@ -158,7 +158,7 @@ LL | fn test11(x: &usize) -> &_ { | -^ | || | |not allowed in type signatures - | help: replace with the correct return type: `&'static &'static usize` + | help: replace with the correct return type: `&&usize` error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types --> $DIR/typeck_type_placeholder_item.rs:53:52 @@ -687,13 +687,7 @@ help: add `#![feature(const_trait_impl)]` to the crate attributes to enable LL + #![feature(const_trait_impl)] | -error[E0515]: cannot return reference to function parameter `x` - --> $DIR/typeck_type_placeholder_item.rs:50:5 - | -LL | &x - | ^^ returns a reference to data owned by the current function - -error: aborting due to 75 previous errors +error: aborting due to 74 previous errors -Some errors have detailed explanations: E0015, E0046, E0121, E0282, E0403, E0515. +Some errors have detailed explanations: E0015, E0046, E0121, E0282, E0403. For more information about an error, try `rustc --explain E0015`. From 4442fd7a0985ad3ac219b4f9ce8f26a27500acac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Fri, 28 Jun 2024 12:18:32 +0200 Subject: [PATCH 13/19] Add a run-make test that LLD is not being used by default on the x64 beta/stable channel --- .../rust-lld-by-default-beta-stable/main.rs | 1 + .../rust-lld-by-default-beta-stable/rmake.rs | 29 +++++++++++++++++++ .../main.rs | 0 .../rmake.rs | 4 +-- 4 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 tests/run-make/rust-lld-by-default-beta-stable/main.rs create mode 100644 tests/run-make/rust-lld-by-default-beta-stable/rmake.rs rename tests/run-make/{rust-lld-by-default => rust-lld-by-default-nightly}/main.rs (100%) rename tests/run-make/{rust-lld-by-default => rust-lld-by-default-nightly}/rmake.rs (93%) diff --git a/tests/run-make/rust-lld-by-default-beta-stable/main.rs b/tests/run-make/rust-lld-by-default-beta-stable/main.rs new file mode 100644 index 0000000000000..f328e4d9d04c3 --- /dev/null +++ b/tests/run-make/rust-lld-by-default-beta-stable/main.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/tests/run-make/rust-lld-by-default-beta-stable/rmake.rs b/tests/run-make/rust-lld-by-default-beta-stable/rmake.rs new file mode 100644 index 0000000000000..fc3dffdbaf42a --- /dev/null +++ b/tests/run-make/rust-lld-by-default-beta-stable/rmake.rs @@ -0,0 +1,29 @@ +// Ensure that rust-lld is *not* used as the default linker on `x86_64-unknown-linux-gnu` on stable +// or beta. + +//@ ignore-nightly +//@ only-x86_64-unknown-linux-gnu + +use run_make_support::regex::Regex; +use run_make_support::rustc; +use std::process::Output; + +fn main() { + // A regular compilation should not use rust-lld by default. We'll check that by asking the + // linker to display its version number with a link-arg. + let output = rustc() + .env("RUSTC_LOG", "rustc_codegen_ssa::back::link=info") + .link_arg("-Wl,-v") + .input("main.rs") + .run(); + assert!( + !find_lld_version_in_logs(output.stderr_utf8()), + "the LLD version string should not be present in the output logs:\n{}", + output.stderr_utf8() + ); +} + +fn find_lld_version_in_logs(stderr: String) -> bool { + let lld_version_re = Regex::new(r"^LLD [0-9]+\.[0-9]+\.[0-9]+").unwrap(); + stderr.lines().any(|line| lld_version_re.is_match(line.trim())) +} diff --git a/tests/run-make/rust-lld-by-default/main.rs b/tests/run-make/rust-lld-by-default-nightly/main.rs similarity index 100% rename from tests/run-make/rust-lld-by-default/main.rs rename to tests/run-make/rust-lld-by-default-nightly/main.rs diff --git a/tests/run-make/rust-lld-by-default/rmake.rs b/tests/run-make/rust-lld-by-default-nightly/rmake.rs similarity index 93% rename from tests/run-make/rust-lld-by-default/rmake.rs rename to tests/run-make/rust-lld-by-default-nightly/rmake.rs index 94857a57dfb7c..f3ce9ada15709 100644 --- a/tests/run-make/rust-lld-by-default/rmake.rs +++ b/tests/run-make/rust-lld-by-default-nightly/rmake.rs @@ -1,5 +1,5 @@ -// Ensure that rust-lld is used as the default linker on `x86_64-unknown-linux-gnu`, and that it can -// also be turned off with a CLI flag. +// Ensure that rust-lld is used as the default linker on `x86_64-unknown-linux-gnu` on the nightly +// channel, and that it can also be turned off with a CLI flag. //@ needs-rust-lld //@ ignore-beta From 9c0ce05d246811a977b6564b7df2b6947bd6903b Mon Sep 17 00:00:00 2001 From: surechen Date: Sat, 29 Jun 2024 18:45:07 +0800 Subject: [PATCH 14/19] Show `used attribute`'s kind for user when find it isn't applied to a `static` variable. fixes #126789 --- compiler/rustc_passes/messages.ftl | 1 + compiler/rustc_passes/src/check_attr.rs | 10 +++++++--- compiler/rustc_passes/src/errors.rs | 3 +++ tests/ui/attributes/used-issue-126789.rs | 6 ++++++ tests/ui/attributes/used-issue-126789.stderr | 10 ++++++++++ tests/ui/used.stderr | 8 ++++++++ 6 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 tests/ui/attributes/used-issue-126789.rs create mode 100644 tests/ui/attributes/used-issue-126789.stderr diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index 07c82065a80d1..3cc909a9d6e45 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -786,6 +786,7 @@ passes_used_compiler_linker = passes_used_static = attribute must be applied to a `static` variable + .label = but this is a {$target} passes_useless_assignment = useless assignment of {$is_field_assign -> diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index a0b3470df6dba..238e40022c9cf 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -278,7 +278,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } self.check_repr(attrs, span, target, item, hir_id); - self.check_used(attrs, target); + self.check_used(attrs, target, span); } fn inline_attr_str_error_with_macro_def(&self, hir_id: HirId, attr: &Attribute, sym: &str) { @@ -1978,12 +1978,16 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } } - fn check_used(&self, attrs: &[Attribute], target: Target) { + fn check_used(&self, attrs: &[Attribute], target: Target, target_span: Span) { let mut used_linker_span = None; let mut used_compiler_span = None; for attr in attrs.iter().filter(|attr| attr.has_name(sym::used)) { if target != Target::Static { - self.dcx().emit_err(errors::UsedStatic { span: attr.span }); + self.dcx().emit_err(errors::UsedStatic { + attr_span: attr.span, + span: target_span, + target: target.name(), + }); } let inner = attr.meta_item_list(); match inner.as_deref() { diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 25df80d5a92ca..897a377ce3461 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -563,7 +563,10 @@ pub struct ReprConflictingLint; #[diag(passes_used_static)] pub struct UsedStatic { #[primary_span] + pub attr_span: Span, + #[label] pub span: Span, + pub target: &'static str, } #[derive(Diagnostic)] diff --git a/tests/ui/attributes/used-issue-126789.rs b/tests/ui/attributes/used-issue-126789.rs new file mode 100644 index 0000000000000..90a1aa8d5cc3d --- /dev/null +++ b/tests/ui/attributes/used-issue-126789.rs @@ -0,0 +1,6 @@ +extern "C" { + #[used] //~ ERROR attribute must be applied to a `static` variable + static FOO: i32; +} + +fn main() {} diff --git a/tests/ui/attributes/used-issue-126789.stderr b/tests/ui/attributes/used-issue-126789.stderr new file mode 100644 index 0000000000000..6014f7af95c55 --- /dev/null +++ b/tests/ui/attributes/used-issue-126789.stderr @@ -0,0 +1,10 @@ +error: attribute must be applied to a `static` variable + --> $DIR/used-issue-126789.rs:2:5 + | +LL | #[used] + | ^^^^^^^ +LL | static FOO: i32; + | ---------------- but this is a foreign static item + +error: aborting due to 1 previous error + diff --git a/tests/ui/used.stderr b/tests/ui/used.stderr index ea77f129d8ef0..c586dc722932f 100644 --- a/tests/ui/used.stderr +++ b/tests/ui/used.stderr @@ -3,24 +3,32 @@ error: attribute must be applied to a `static` variable | LL | #[used] | ^^^^^^^ +LL | fn foo() {} + | ----------- but this is a function error: attribute must be applied to a `static` variable --> $DIR/used.rs:7:1 | LL | #[used] | ^^^^^^^ +LL | struct Foo {} + | ------------- but this is a struct error: attribute must be applied to a `static` variable --> $DIR/used.rs:10:1 | LL | #[used] | ^^^^^^^ +LL | trait Bar {} + | ------------ but this is a trait error: attribute must be applied to a `static` variable --> $DIR/used.rs:13:1 | LL | #[used] | ^^^^^^^ +LL | impl Bar for Foo {} + | ------------------- but this is a implementation block error: aborting due to 4 previous errors From 8dc36c16470d60fa1bd7bc370c77ac63ef25bfe4 Mon Sep 17 00:00:00 2001 From: Lin Yihai Date: Sat, 29 Jun 2024 16:49:35 +0800 Subject: [PATCH 15/19] fix: prefer `(*p).clone` to `p.clone` if the `p` is a raw pointer --- .../src/diagnostics/conflict_errors.rs | 26 +++++++++++++++--- .../src/diagnostics/move_errors.rs | 27 ++++++++++++++----- .../borrowck-move-from-unsafe-ptr.stderr | 10 ++----- tests/ui/borrowck/issue-20801.stderr | 10 ------- .../move-from-union-field-issue-66500.stderr | 10 +++---- 5 files changed, 50 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 1cc7fee718e87..c26bbb926eaa1 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -1288,7 +1288,7 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> { return false; } // Try to find predicates on *generic params* that would allow copying `ty` - let suggestion = + let mut suggestion = if let Some(symbol) = tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) { format!(": {symbol}.clone()") } else { @@ -1296,6 +1296,8 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> { }; let mut sugg = Vec::with_capacity(2); let mut inner_expr = expr; + let mut is_raw_ptr = false; + let typeck_result = self.infcx.tcx.typeck(self.mir_def_id()); // Remove uses of `&` and `*` when suggesting `.clone()`. while let hir::ExprKind::AddrOf(.., inner) | hir::ExprKind::Unary(hir::UnOp::Deref, inner) = &inner_expr.kind @@ -1306,14 +1308,32 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> { return false; } inner_expr = inner; + if let Some(inner_type) = typeck_result.node_type_opt(inner.hir_id) { + if matches!(inner_type.kind(), ty::RawPtr(..)) { + is_raw_ptr = true; + break; + } + } } - if inner_expr.span.lo() != expr.span.lo() { + // Cloning the raw pointer doesn't make sense in some cases and would cause a type mismatch error. (see #126863) + if inner_expr.span.lo() != expr.span.lo() && !is_raw_ptr { + // Remove "(*" or "(&" sugg.push((expr.span.with_hi(inner_expr.span.lo()), String::new())); } + // Check whether `expr` is surrounded by parentheses or not. let span = if inner_expr.span.hi() != expr.span.hi() { // Account for `(*x)` to suggest `x.clone()`. - expr.span.with_lo(inner_expr.span.hi()) + if is_raw_ptr { + expr.span.shrink_to_hi() + } else { + // Remove the close parenthesis ")" + expr.span.with_lo(inner_expr.span.hi()) + } } else { + if is_raw_ptr { + sugg.push((expr.span.shrink_to_lo(), "(".to_string())); + suggestion = ").clone()".to_string(); + } expr.span.shrink_to_hi() }; sugg.push((span, suggestion)); diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs index 12fa4c4f5ee05..407b83d497793 100644 --- a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs @@ -639,12 +639,27 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> { fn add_borrow_suggestions(&self, err: &mut Diag<'_>, span: Span) { match self.infcx.tcx.sess.source_map().span_to_snippet(span) { Ok(snippet) if snippet.starts_with('*') => { - err.span_suggestion_verbose( - span.with_hi(span.lo() + BytePos(1)), - "consider removing the dereference here", - String::new(), - Applicability::MaybeIncorrect, - ); + let sp = span.with_lo(span.lo() + BytePos(1)); + let inner = self.find_expr(sp); + let mut is_raw_ptr = false; + if let Some(inner) = inner { + let typck_result = self.infcx.tcx.typeck(self.mir_def_id()); + if let Some(inner_type) = typck_result.node_type_opt(inner.hir_id) { + if matches!(inner_type.kind(), ty::RawPtr(..)) { + is_raw_ptr = true; + } + } + } + // If the `inner` is a raw pointer, do not suggest removing the "*", see #126863 + // FIXME: need to check whether the assigned object can be a raw pointer, see `tests/ui/borrowck/issue-20801.rs`. + if !is_raw_ptr { + err.span_suggestion_verbose( + span.with_hi(span.lo() + BytePos(1)), + "consider removing the dereference here", + String::new(), + Applicability::MaybeIncorrect, + ); + } } _ => { err.span_suggestion_verbose( diff --git a/tests/ui/borrowck/borrowck-move-from-unsafe-ptr.stderr b/tests/ui/borrowck/borrowck-move-from-unsafe-ptr.stderr index ebc3b6ebcacda..79f18624c618b 100644 --- a/tests/ui/borrowck/borrowck-move-from-unsafe-ptr.stderr +++ b/tests/ui/borrowck/borrowck-move-from-unsafe-ptr.stderr @@ -4,16 +4,10 @@ error[E0507]: cannot move out of `*x` which is behind a raw pointer LL | let y = *x; | ^^ move occurs because `*x` has type `Box`, which does not implement the `Copy` trait | -help: consider removing the dereference here - | -LL - let y = *x; -LL + let y = x; - | help: consider cloning the value if the performance cost is acceptable | -LL - let y = *x; -LL + let y = x.clone(); - | +LL | let y = (*x).clone(); + | + +++++++++ error: aborting due to 1 previous error diff --git a/tests/ui/borrowck/issue-20801.stderr b/tests/ui/borrowck/issue-20801.stderr index 20a4bd4e42378..c1d06ac3e2196 100644 --- a/tests/ui/borrowck/issue-20801.stderr +++ b/tests/ui/borrowck/issue-20801.stderr @@ -67,11 +67,6 @@ LL | struct T(u8); ... LL | let c = unsafe { *mut_ptr() }; | ---------- you could clone this value -help: consider removing the dereference here - | -LL - let c = unsafe { *mut_ptr() }; -LL + let c = unsafe { mut_ptr() }; - | error[E0507]: cannot move out of a raw pointer --> $DIR/issue-20801.rs:36:22 @@ -87,11 +82,6 @@ LL | struct T(u8); ... LL | let d = unsafe { *const_ptr() }; | ------------ you could clone this value -help: consider removing the dereference here - | -LL - let d = unsafe { *const_ptr() }; -LL + let d = unsafe { const_ptr() }; - | error: aborting due to 4 previous errors; 1 warning emitted diff --git a/tests/ui/borrowck/move-from-union-field-issue-66500.stderr b/tests/ui/borrowck/move-from-union-field-issue-66500.stderr index c951ce8e3cd8f..7f4593eefcaf6 100644 --- a/tests/ui/borrowck/move-from-union-field-issue-66500.stderr +++ b/tests/ui/borrowck/move-from-union-field-issue-66500.stderr @@ -30,9 +30,8 @@ LL | *u.c | help: consider cloning the value if the performance cost is acceptable | -LL - *u.c -LL + u.c.clone() - | +LL | (*u.c).clone() + | + +++++++++ error[E0507]: cannot move out of `*u.d` which is behind a raw pointer --> $DIR/move-from-union-field-issue-66500.rs:24:5 @@ -42,9 +41,8 @@ LL | *u.d | help: consider cloning the value if the performance cost is acceptable | -LL - *u.d -LL + u.d.clone() - | +LL | (*u.d).clone() + | + +++++++++ error: aborting due to 4 previous errors From c54a2a53f83d9eb4fec161ecc304164d0303fbfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 27 Jun 2024 19:03:47 +0200 Subject: [PATCH 16/19] Make mtime of reproducible tarball dependent on git commit --- src/bootstrap/src/utils/tarball.rs | 26 ++++++++++++++++++++++- src/tools/rust-installer/src/tarballer.rs | 18 ++++++++++++++-- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/bootstrap/src/utils/tarball.rs b/src/bootstrap/src/utils/tarball.rs index 5cc319826dbf3..3c15bb296f39a 100644 --- a/src/bootstrap/src/utils/tarball.rs +++ b/src/bootstrap/src/utils/tarball.rs @@ -9,9 +9,9 @@ use std::path::{Path, PathBuf}; use crate::core::builder::Builder; use crate::core::{build_steps::dist::distdir, builder::Kind}; -use crate::utils::channel; use crate::utils::exec::BootstrapCommand; use crate::utils::helpers::{move_file, t}; +use crate::utils::{channel, helpers}; #[derive(Copy, Clone)] pub(crate) enum OverlayKind { @@ -351,6 +351,30 @@ impl<'a> Tarball<'a> { }; cmd.args(["--compression-profile", compression_profile]); + + // We want to use a pinned modification time for files in the archive + // to achieve better reproducibility. However, using the same mtime for all + // releases is not ideal, because it can break e.g. Cargo mtime checking + // (https://github.com/rust-lang/rust/issues/125578). + // Therefore, we set mtime to the date of the latest commit (if we're managed + // by git). In this way, the archive will still be always the same for a given commit + // (achieving reproducibility), but it will also change between different commits and + // Rust versions, so that it won't break mtime-based caches. + // + // Note that this only overrides the mtime of files, not directories, due to the + // limitations of the tarballer tool. Directories will have their mtime set to 2006. + + // Get the UTC timestamp of the last git commit, if we're under git. + // We need to use UTC, so that anyone who tries to rebuild from the same commit + // gets the same timestamp. + if self.builder.rust_info().is_managed_git_subrepository() { + // %ct means committer date + let timestamp = helpers::output( + helpers::git(Some(&self.builder.src)).arg("log").arg("-1").arg("--format=%ct"), + ); + cmd.args(["--override-file-mtime", timestamp.trim()]); + } + self.builder.run(cmd); // Ensure there are no symbolic links in the tarball. In particular, diff --git a/src/tools/rust-installer/src/tarballer.rs b/src/tools/rust-installer/src/tarballer.rs index 2f093e7ad17fd..b5e87a66ffc39 100644 --- a/src/tools/rust-installer/src/tarballer.rs +++ b/src/tools/rust-installer/src/tarballer.rs @@ -32,6 +32,12 @@ actor! { /// The formats used to compress the tarball. #[arg(value_name = "FORMAT", default_value_t)] compression_formats: CompressionFormats, + + /// Modification time that will be set for all files added to the archive. + /// The default is the date of the first Rust commit from 2006. + /// This serves for better reproducibility of the archives. + #[arg(value_name = "FILE_MTIME", default_value_t = 1153704088)] + override_file_mtime: u64, } } @@ -65,6 +71,8 @@ impl Tarballer { let buf = BufWriter::with_capacity(1024 * 1024, encoder); let mut builder = Builder::new(buf); // Make uid, gid and mtime deterministic to improve reproducibility + // The modification time of directories will be set to the date of the first Rust commit. + // The modification time of files will be set to `override_file_mtime` (see `append_path`). builder.mode(HeaderMode::Deterministic); let pool = rayon::ThreadPoolBuilder::new().num_threads(2).build().unwrap(); @@ -77,7 +85,7 @@ impl Tarballer { } for path in files { let src = Path::new(&self.work_dir).join(&path); - append_path(&mut builder, &src, &path) + append_path(&mut builder, &src, &path, self.override_file_mtime) .with_context(|| format!("failed to tar file '{}'", src.display()))?; } builder @@ -93,10 +101,16 @@ impl Tarballer { } } -fn append_path(builder: &mut Builder, src: &Path, path: &String) -> Result<()> { +fn append_path( + builder: &mut Builder, + src: &Path, + path: &String, + override_file_mtime: u64, +) -> Result<()> { let stat = symlink_metadata(src)?; let mut header = Header::new_gnu(); header.set_metadata_in_mode(&stat, HeaderMode::Deterministic); + header.set_mtime(override_file_mtime); if stat.file_type().is_symlink() { let link = read_link(src)?; From f6f21a8f11a51336784e93d6ea712f4484d7caef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 29 Jun 2024 16:07:22 +0200 Subject: [PATCH 17/19] Review changes --- src/bootstrap/src/core/build_steps/perf.rs | 2 +- src/tools/rustc-perf-wrapper/README.md | 2 +- src/tools/rustc-perf-wrapper/src/main.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/perf.rs b/src/bootstrap/src/core/build_steps/perf.rs index f8170580722bf..f41b5fe10f1d9 100644 --- a/src/bootstrap/src/core/build_steps/perf.rs +++ b/src/bootstrap/src/core/build_steps/perf.rs @@ -27,7 +27,7 @@ Consider setting `rust.debuginfo-level = 1` in `config.toml`."#); let args = std::env::args().skip_while(|a| a != "--").skip(1); let mut cmd = builder.tool_cmd(Tool::RustcPerfWrapper); - cmd.env("PERF_RUSTC", rustc) + cmd.env("RUSTC_REAL", rustc) .env("PERF_COLLECTOR", collector) .env("PERF_RESULT_DIR", profile_results_dir) .args(args); diff --git a/src/tools/rustc-perf-wrapper/README.md b/src/tools/rustc-perf-wrapper/README.md index 7c096e3081416..d7655459a2fe1 100644 --- a/src/tools/rustc-perf-wrapper/README.md +++ b/src/tools/rustc-perf-wrapper/README.md @@ -1,3 +1,3 @@ # rustc-perf wrapper Utility tool for invoking [`rustc-perf`](https://github.com/rust-lang/rustc-perf) for benchmarking/profiling -a stage1/2 compiler built by bootstrap using `x run perf`. +a stage1/2 compiler built by bootstrap using `x perf -- `. diff --git a/src/tools/rustc-perf-wrapper/src/main.rs b/src/tools/rustc-perf-wrapper/src/main.rs index 0974661f99787..1c0d1745f3d98 100644 --- a/src/tools/rustc-perf-wrapper/src/main.rs +++ b/src/tools/rustc-perf-wrapper/src/main.rs @@ -68,7 +68,7 @@ struct SharedOpts { #[derive(Debug, clap::Parser)] struct BuildContext { /// Compiler binary that will be benchmarked/profiled. - #[clap(long, hide = true, env = "PERF_RUSTC")] + #[clap(long, hide = true, env = "RUSTC_REAL")] compiler: PathBuf, /// rustc-perf collector binary that will be used for running benchmarks/profilers. #[clap(long, hide = true, env = "PERF_COLLECTOR")] From 6a2638e6c49817de9c2b1d56261de37c6a352571 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Sat, 29 Jun 2024 16:07:39 +0200 Subject: [PATCH 18/19] Autolabel `rustc-perf-wrapper` changes with t-bootstrap label --- triagebot.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/triagebot.toml b/triagebot.toml index 62e0917efabed..8ae454d412a6f 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -331,6 +331,7 @@ trigger_files = [ "src/tools/tidy", "src/tools/rustdoc-gui-test", "src/tools/libcxx-version", + "src/tools/rustc-perf-wrapper", ] [autolabel."T-infra"] From 15d5dac32ecd54745d6f61659628286bd2678e03 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 28 Jun 2024 23:14:09 -0300 Subject: [PATCH 19/19] Avoid suggesting to add unsafe when the extern block is already unsafe --- .../rustc_ast_passes/src/ast_validation.rs | 19 ++++++++++++------- compiler/rustc_ast_passes/src/errors.rs | 2 +- tests/ui/parser/unsafe-foreign-mod-2.stderr | 5 ----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index d02b8510975fc..60690a7e2cd83 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -459,13 +459,18 @@ impl<'a> AstValidator<'a> { fn check_item_safety(&self, span: Span, safety: Safety) { match self.extern_mod_safety { Some(extern_safety) => { - if matches!(safety, Safety::Unsafe(_) | Safety::Safe(_)) - && (extern_safety == Safety::Default || !self.features.unsafe_extern_blocks) - { - self.dcx().emit_err(errors::InvalidSafetyOnExtern { - item_span: span, - block: self.current_extern_span().shrink_to_lo(), - }); + if matches!(safety, Safety::Unsafe(_) | Safety::Safe(_)) { + if extern_safety == Safety::Default { + self.dcx().emit_err(errors::InvalidSafetyOnExtern { + item_span: span, + block: Some(self.current_extern_span().shrink_to_lo()), + }); + } else if !self.features.unsafe_extern_blocks { + self.dcx().emit_err(errors::InvalidSafetyOnExtern { + item_span: span, + block: None, + }); + } } } None => { diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs index 965d8fac712ae..bfb9047645011 100644 --- a/compiler/rustc_ast_passes/src/errors.rs +++ b/compiler/rustc_ast_passes/src/errors.rs @@ -222,7 +222,7 @@ pub struct InvalidSafetyOnExtern { #[primary_span] pub item_span: Span, #[suggestion(code = "unsafe ", applicability = "machine-applicable", style = "verbose")] - pub block: Span, + pub block: Option, } #[derive(Diagnostic)] diff --git a/tests/ui/parser/unsafe-foreign-mod-2.stderr b/tests/ui/parser/unsafe-foreign-mod-2.stderr index 07dbd5568d053..8bd592b5d4311 100644 --- a/tests/ui/parser/unsafe-foreign-mod-2.stderr +++ b/tests/ui/parser/unsafe-foreign-mod-2.stderr @@ -19,11 +19,6 @@ error: items in unadorned `extern` blocks cannot have safety qualifiers | LL | unsafe fn foo(); | ^^^^^^^^^^^^^^^^ - | -help: add unsafe to this `extern` block - | -LL | unsafe extern "C" unsafe { - | ++++++ error: aborting due to 3 previous errors