From 9783bec6d3797db603fffdac389af1cb8707ad96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Bernier?= Date: Thu, 10 Sep 2026 23:40:22 -0400 Subject: [PATCH 1/3] Fix nested always-inline call chains Only drop #[inline(always)] to a plain inline hint when GCC can't honor it. Before, any always-inline function that called another one was demoted, so at -O0 acyclic wrapper chains (intrinsic wrappers in particular) became real calls. always_inline isn't a hint for GCC: it fails the build when it can't inline. So a function keeps it unless: - it is only declared in this codegen unit; - it uses va_arg; - an x86-interrupt handler here could inline it. Handlers are built with general-regs-only, and GCC won't inline normally-built code into them; - a foreign declaration can call it: an extern item importing its exported symbol, or the EII declaration it implements. GCC sees those as direct calls, but the Rust call graph doesn't; - GCC could end up inlining it into itself. All but the last also apply to #[rustc_force_inline]. The last check runs on a call graph over monomorphized instances, taken from the collector's items_of_instance, so trait methods and closures resolve to their real bodies. Besides calls, an edge also goes to every function whose address is taken: fn pointers, vtable methods, constants and static initializers. GCC turns calls through a known address into direct calls, at -O0 for an immediate &f and from -O1 through read-only tables and vtables. At -O0 only always-inline functions are nodes; when optimizing, anything not marked noinline is, since GCC may inline plain helpers too. Imported monomorphizations are leaves; querying them would also check them against the wrong target features. Cycles are found with rustc's graph::scc::Sccs over the unclassified part of the graph, and every function visited is cached per codegen unit. Only functions on a cycle are demoted, so callers that merely reach one keep always_inline. When optimizing, recursing through a plain helper also demotes. A regex release build takes the same time as before (41.3-41.6 s). requested_inline is shared by from_fn_attrs and the graph, so both see the same effective attribute (naked, requires_inline, weak linkage). from_fn_attrs now takes an FnBody saying whether the function gets a body. Tests: - tests/run/always_inline.rs: cycles through a trait method, an FnMut closure and a local fn pointer. - tests/asm: an acyclic chain at -O0, a caller of a recursive function keeping always_inline, cycles through a static fn table and a vtable at -O1, and an upstream avx monomorphization reached through a static table. The asm tester drops the build system's --out-dir so compiletest's aux-build works. - tests/compile: a variadic function, a foreign declaration, a link_name alias, an EII implementation calling its declaration, and x86-interrupt handlers. The variadic and interrupt tests are skipped on m68k. mini_core gets the unsafe_unpin lang item, which current rustc needs to optimize generic &mut F at -O3. --- build_system/asm-tester/src/main.rs | 12 +- example/mini_core.rs | 9 + src/attributes.rs | 342 +++++++++++++++---- src/callee.rs | 4 +- src/context.rs | 6 + src/intrinsic/mod.rs | 8 +- src/mono_item.rs | 3 +- tests/asm/always_inline.rs | 31 ++ tests/asm/always_inline_address_cycles.rs | 53 +++ tests/asm/always_inline_cycle_caller.rs | 27 ++ tests/asm/always_inline_imported_mono.rs | 25 ++ tests/asm/auxiliary/avx_generic.rs | 21 ++ tests/compile/always_inline_eii.rs | 29 ++ tests/compile/always_inline_extern_decl.rs | 15 + tests/compile/always_inline_symbol_alias.rs | 29 ++ tests/compile/always_inline_variadic.rs | 19 ++ tests/compile/always_inline_x86_interrupt.rs | 27 ++ tests/lang_tests.rs | 3 + tests/run/always_inline.rs | 66 +++- 19 files changed, 663 insertions(+), 66 deletions(-) create mode 100644 tests/asm/always_inline.rs create mode 100644 tests/asm/always_inline_address_cycles.rs create mode 100644 tests/asm/always_inline_cycle_caller.rs create mode 100644 tests/asm/always_inline_imported_mono.rs create mode 100644 tests/asm/auxiliary/avx_generic.rs create mode 100644 tests/compile/always_inline_eii.rs create mode 100644 tests/compile/always_inline_extern_decl.rs create mode 100644 tests/compile/always_inline_symbol_alias.rs create mode 100644 tests/compile/always_inline_variadic.rs create mode 100644 tests/compile/always_inline_x86_interrupt.rs diff --git a/build_system/asm-tester/src/main.rs b/build_system/asm-tester/src/main.rs index 00ee4ac9365..fc6df1f5930 100644 --- a/build_system/asm-tester/src/main.rs +++ b/build_system/asm-tester/src/main.rs @@ -24,9 +24,15 @@ impl Config { } } "--" => { - config.rustc_flags.extend(&mut args); - // Nothing else to be read but the `break` makes it more clear. - break; + // compiletest passes its own `--out-dir` to auxiliary builds, and rustc + // rejects a second one. + while let Some(arg) = args.next() { + if arg == "--out-dir" { + args.next(); + } else { + config.rustc_flags.push(arg); + } + } } arg => return Err(format!("Unknown argument {arg:?}")), } diff --git a/example/mini_core.rs b/example/mini_core.rs index 2d5a29ceb81..2827d003fe7 100644 --- a/example/mini_core.rs +++ b/example/mini_core.rs @@ -145,6 +145,15 @@ unsafe impl Freeze for *mut T {} unsafe impl Freeze for &T {} unsafe impl Freeze for &mut T {} +#[lang = "unsafe_unpin"] +pub unsafe auto trait UnsafeUnpin {} + +unsafe impl UnsafeUnpin for PhantomData {} +unsafe impl UnsafeUnpin for *const T {} +unsafe impl UnsafeUnpin for *mut T {} +unsafe impl UnsafeUnpin for &T {} +unsafe impl UnsafeUnpin for &mut T {} + #[lang = "structural_peq"] pub trait StructuralPartialEq {} diff --git a/src/attributes.rs b/src/attributes.rs index 41db5e83bdc..d8002e14146 100644 --- a/src/attributes.rs +++ b/src/attributes.rs @@ -1,16 +1,37 @@ +#[cfg(feature = "master")] +use std::cell::{OnceCell, RefCell}; +#[cfg(feature = "master")] +use std::iter; + #[cfg(feature = "master")] use gccjit::FnAttribute; use gccjit::Function; #[cfg(feature = "master")] use rustc_abi::{CanonAbi, InterruptKind}; #[cfg(feature = "master")] +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +#[cfg(feature = "master")] +use rustc_data_structures::graph::scc::Sccs; +#[cfg(feature = "master")] +use rustc_data_structures::graph::vec_graph::VecGraph; +#[cfg(feature = "master")] use rustc_hir::attrs::InlineAttr; use rustc_hir::attrs::InstructionSetAttr; #[cfg(feature = "master")] -use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; +use rustc_hir::def::DefKind; +#[cfg(feature = "master")] +use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; #[cfg(feature = "master")] -use rustc_middle::mir::TerminatorKind; +use rustc_middle::mir::interpret::{AllocId, GlobalAlloc}; +#[cfg(feature = "master")] +use rustc_middle::mono::{CollectionMode, MonoItem}; use rustc_middle::ty; +#[cfg(feature = "master")] +use rustc_middle::ty::layout::FnAbiOf; +#[cfg(feature = "master")] +use rustc_session::config::OptLevel; +#[cfg(feature = "master")] +use rustc_span::def_id::{DefId, LOCAL_CRATE}; use rustc_target::callconv::FnAbi; #[cfg(feature = "master")] use rustc_target::spec::Arch; @@ -20,34 +41,257 @@ use crate::base; use crate::context::CodegenCx; use crate::gcc_util::to_gcc_features; -/// Checks if the function `instance` is recursively inline. -/// Returns `false` if a functions is guaranteed to be non-recursive, and `true` if it *might* be recursive. +/// Whether the GCC function being annotated gets a body in this codegen unit. +#[derive(Clone, Copy)] +pub enum FnBody { + Defined, + Declared, +} + +/// What we need to know, per codegen unit, to decide where `always_inline` is safe. +#[cfg(feature = "master")] +#[derive(Default)] +pub struct InlineAnalysis<'tcx> { + /// Whether GCC could end up inlining the instance into itself. + in_cycle: RefCell, bool>>, + interrupt_callees: OnceCell>>, + foreign_imports: OnceCell>, +} + +/// The inlining we ask GCC for, before checking whether it can honor `always_inline`. +#[cfg(feature = "master")] +fn requested_inline<'tcx>( + tcx: ty::TyCtxt<'tcx>, + instance: ty::Instance<'tcx>, + attrs: &CodegenFnAttrs, +) -> InlineAttr { + let inline = if attrs.flags.contains(CodegenFnAttrFlags::NAKED) { + InlineAttr::Never + } else if attrs.inline == InlineAttr::None && instance.def.requires_inline(tcx) { + InlineAttr::Hint + } else { + attrs.inline + }; + // GCC drops `weak` from a function that is also `inline`, leaving the symbol strong, and + // the linkage is what has to survive. `inline(never)` does not conflict. + match inline { + InlineAttr::Always | InlineAttr::Hint | InlineAttr::Force { .. } + if attrs.linkage.is_some_and(base::linkage_needs_weak_attribute) => + { + InlineAttr::None + } + inline => inline, + } +} + +/// Whether GCC may inline `instance` into a caller. At `-O0` it only inlines `always_inline` +/// functions; above that, anything not marked `noinline`. +#[cfg(feature = "master")] +fn gcc_may_inline<'tcx>( + tcx: ty::TyCtxt<'tcx>, + instance: ty::Instance<'tcx>, + optimize: OptLevel, +) -> bool { + match requested_inline(tcx, instance, &tcx.codegen_instance_attrs(instance.def)) { + InlineAttr::Always | InlineAttr::Force { .. } => true, + InlineAttr::Never => false, + InlineAttr::Hint | InlineAttr::None => optimize != OptLevel::No, + } +} + +/// Functions whose code GCC could pull into `instance`: its callees, plus anything whose address +/// it takes (fn pointers, vtables, statics), since GCC turns calls through a known address into +/// direct calls and inlines those too. +#[cfg(feature = "master")] +fn inline_edges<'tcx>( + tcx: ty::TyCtxt<'tcx>, + instance: ty::Instance<'tcx>, + optimize: OptLevel, +) -> Vec> { + // Nothing to inline without a body. Monomorphizations we import from another crate count as + // bodyless: querying them would also check them against the wrong target features. + let has_body = match instance.def { + ty::InstanceKind::Item(def_id) => tcx.is_mir_available(def_id), + ty::InstanceKind::Intrinsic(_) + | ty::InstanceKind::LlvmIntrinsic(_) + | ty::InstanceKind::Virtual(..) => false, + ty::InstanceKind::Shim(_) => true, + }; + let mut edges = Vec::new(); + if !has_body || !tcx.should_codegen_locally(instance) { + return edges; + } + // The collector already resolved all of this, and reported its errors. + let Ok((used, _)) = tcx.items_of_instance((instance, CollectionMode::UsedItems)) else { + return edges; + }; + for item in used { + match item.node { + MonoItem::Fn(callee) => edges.push(callee), + MonoItem::Static(def_id) => static_fn_addresses(tcx, def_id, &mut edges), + MonoItem::GlobalAsm(_) => {} + } + } + edges.retain(|&callee| gcc_may_inline(tcx, callee, optimize)); + edges +} + +/// Functions reachable through the initializer of `def_id`. GCC can fold loads from read-only +/// data, so a call through a static fn table can become a direct call. +#[cfg(feature = "master")] +fn static_fn_addresses<'tcx>( + tcx: ty::TyCtxt<'tcx>, + def_id: DefId, + out: &mut Vec>, +) { + fn initializer_ptrs(tcx: ty::TyCtxt<'_>, def_id: DefId, pending: &mut Vec) { + if tcx.is_foreign_item(def_id) + || !tcx.should_codegen_locally(ty::Instance::mono(tcx, def_id)) + { + return; + } + if let Ok(alloc) = tcx.eval_static_initializer(def_id) { + pending.extend(alloc.inner().provenance().ptrs().values().map(|prov| prov.alloc_id())); + } + } + + let mut pending = Vec::new(); + initializer_ptrs(tcx, def_id, &mut pending); + let mut seen = FxHashSet::default(); + while let Some(alloc_id) = pending.pop() { + if !seen.insert(alloc_id) { + continue; + } + match tcx.global_alloc(alloc_id) { + GlobalAlloc::Function { instance, .. } => { + if tcx.should_codegen_locally(instance) { + out.push(instance); + } + } + GlobalAlloc::Memory(alloc) => pending + .extend(alloc.inner().provenance().ptrs().values().map(|prov| prov.alloc_id())), + GlobalAlloc::VTable(ty, dyn_ty) => pending.push( + tcx.vtable_allocation(( + ty, + dyn_ty + .principal() + .map(|principal| tcx.instantiate_bound_regions_with_erased(principal)), + )), + ), + GlobalAlloc::Static(nested) => initializer_ptrs(tcx, nested, &mut pending), + GlobalAlloc::TypeId { .. } => {} + } + } +} + +/// Whether GCC could end up inlining `root` into itself. +/// +/// Classifies everything reachable from `root` at once. Only functions on a cycle are affected: +/// once they lose `always_inline`, callers that merely reach the cycle can keep it. When +/// optimizing, cycles through plain helpers count too, since GCC may inline those as well. +#[cfg(feature = "master")] +fn in_inline_cycle<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, root: ty::Instance<'tcx>) -> bool { + if let Some(&in_cycle) = cx.inline_analysis.in_cycle.borrow().get(&root) { + return in_cycle; + } + + // Collect the part of the graph reachable from `root` that hasn't been classified yet. + // Classified functions can be left out: their SCC is complete, so nothing new can join it. + let optimize = cx.sess().opts.optimize; + let mut nodes = vec![root]; + let mut index = FxHashMap::from_iter([(root, 0)]); + let mut edges = Vec::new(); + { + let classified = cx.inline_analysis.in_cycle.borrow(); + let mut caller = 0; + while let Some(&instance) = nodes.get(caller) { + for callee in inline_edges(cx.tcx, instance, optimize) { + if classified.contains_key(&callee) { + continue; + } + let callee = *index.entry(callee).or_insert_with(|| { + nodes.push(callee); + nodes.len() - 1 + }); + edges.push((caller, callee)); + } + caller += 1; + } + } + + let mut calls_itself = vec![false; nodes.len()]; + for &(caller, callee) in &edges { + calls_itself[caller] |= caller == callee; + } + let sccs: Sccs = Sccs::new(&VecGraph::::new(nodes.len(), edges)); + let mut scc_sizes = vec![0usize; sccs.num_sccs()]; + for node in 0..nodes.len() { + scc_sizes[sccs.scc(node)] += 1; + } + + let mut in_cycle = cx.inline_analysis.in_cycle.borrow_mut(); + for (node, &instance) in nodes.iter().enumerate() { + in_cycle.insert(instance, calls_itself[node] || scc_sizes[sccs.scc(node)] > 1); + } + in_cycle[&root] +} + +/// `always_inline` functions that an `x86-interrupt` handler in this unit could inline. Handlers +/// are built with `general-regs-only`, and GCC refuses to inline normally-built code into them. #[cfg(feature = "master")] -fn recursively_inline<'gcc, 'tcx>( +fn interrupt_callees<'a, 'gcc, 'tcx>( + cx: &'a CodegenCx<'gcc, 'tcx>, +) -> &'a FxHashSet> { + cx.inline_analysis.interrupt_callees.get_or_init(|| { + let optimize = cx.sess().opts.optimize; + cx.codegen_unit + .items() + .keys() + .filter_map(|item| match *item { + MonoItem::Fn(instance) => Some(instance), + MonoItem::Static(_) | MonoItem::GlobalAsm(_) => None, + }) + .filter(|&instance| { + is_x86_interrupt(Some(cx.fn_abi_of_instance(instance, ty::List::empty()))) + }) + .flat_map(|handler| inline_edges(cx.tcx, handler, optimize)) + .filter(|&callee| { + matches!( + requested_inline(cx.tcx, callee, &cx.tcx.codegen_instance_attrs(callee.def)), + InlineAttr::Always | InlineAttr::Force { .. } + ) + }) + .collect() + }) +} + +/// Whether some foreign declaration (`extern` block item or EII declaration) can call `instance`. +/// GCC sees such a call as a direct call to the definition; the Rust call graph doesn't see it. +#[cfg(feature = "master")] +fn called_through_foreign_decl<'gcc, 'tcx>( cx: &CodegenCx<'gcc, 'tcx>, instance: ty::Instance<'tcx>, + attrs: &CodegenFnAttrs, ) -> bool { - // No body, so we can't check if this is recursively inline, so we assume it is. - if !cx.tcx.is_mir_available(instance.def_id()) { + // An EII implementation is always reachable from its declaration, via the forwarding + // wrapper that `add_function_aliases` annotates with this same instance. + if !attrs.foreign_item_symbol_aliases.is_empty() { return true; } - // `expect_local` ought to never fail: we should be checking a function within this codegen unit. - let body = cx.tcx.optimized_mir(instance.def_id()); - for block in body.basic_blocks.iter() { - let Some(ref terminator) = block.terminator else { continue }; - // I assume that the recursive-inline issue applies only to functions, and not to drops. - // In principle, a recursive, `#[inline(always)]` drop could(?) exist, but I don't think it does. - let TerminatorKind::Call { ref func, .. } = terminator.kind else { continue }; - let Some((def, _args)) = func.const_fn_def() else { continue }; - // Check if the called function is recursively inline. - if matches!( - cx.tcx.codegen_fn_attrs(def).inline, - InlineAttr::Always | InlineAttr::Force { .. } - ) { - return true; - } + if !attrs.contains_extern_indicator() { + return false; } - false + let tcx = cx.tcx; + let foreign_imports = cx.inline_analysis.foreign_imports.get_or_init(|| { + iter::once(LOCAL_CRATE) + .chain(tcx.crates(()).iter().copied()) + .flat_map(|krate| tcx.foreign_modules(krate).values()) + .flat_map(|module| &module.foreign_items) + .filter(|&&def_id| tcx.def_kind(def_id) == DefKind::Fn) + .map(|&def_id| tcx.symbol_name(ty::Instance::mono(tcx, def_id)).name) + .collect() + }); + foreign_imports.contains(tcx.symbol_name(instance).name) } /// Get GCC attribute for the provided inline heuristic, attached to `instance`. @@ -57,25 +301,24 @@ fn inline_attr<'gcc, 'tcx>( cx: &CodegenCx<'gcc, 'tcx>, inline: InlineAttr, instance: ty::Instance<'tcx>, + attrs: &CodegenFnAttrs, + body: FnBody, + fn_abi: Option<&FnAbi<'tcx, ty::Ty<'tcx>>>, ) -> Option> { match inline { - InlineAttr::Always => { - // We can't simply always return `always_inline` unconditionally. - // It is *NOT A HINT* and does not work for recursive functions. - // - // So, it can only be applied *if*: - // The current function does not call any functions marked `#[inline(always)]`. - // - // That prevents issues steming from recursive `#[inline(always)]` at a *relatively* small cost. - // We *only* need to check all the terminators of a function marked with this attribute. - if recursively_inline(cx, instance) { - Some(FnAttribute::Inline) - } else { - Some(FnAttribute::AlwaysInline) - } + InlineAttr::Always | InlineAttr::Force { .. } => { + // `always_inline` is *not* a hint: GCC fails the build when it can't inline, so only + // ask for it when it can. + let can_inline = matches!(body, FnBody::Defined) + // GCC never inlines functions that use va_arg. + && !fn_abi.is_some_and(|fn_abi| fn_abi.c_variadic) + && !interrupt_callees(cx).contains(&instance) + && !called_through_foreign_decl(cx, instance, attrs) + // The MIR inliner already rejects `#[rustc_force_inline]` cycles. + && (matches!(inline, InlineAttr::Force { .. }) || !in_inline_cycle(cx, instance)); + if can_inline { Some(FnAttribute::AlwaysInline) } else { Some(FnAttribute::Inline) } } InlineAttr::Hint => Some(FnAttribute::Inline), - InlineAttr::Force { .. } => Some(FnAttribute::AlwaysInline), InlineAttr::Never => { if cx.sess().target.arch != Arch::AmdGpu { Some(FnAttribute::NoInline) @@ -101,6 +344,7 @@ pub fn from_fn_attrs<'gcc, 'tcx>( cx: &CodegenCx<'gcc, 'tcx>, #[cfg_attr(not(feature = "master"), expect(unused_variables))] func: Function<'gcc>, instance: ty::Instance<'tcx>, + #[cfg_attr(not(feature = "master"), expect(unused_variables))] body: FnBody, #[cfg_attr(not(feature = "master"), expect(unused_variables))] fn_abi: Option< &FnAbi<'tcx, ty::Ty<'tcx>>, >, @@ -109,26 +353,8 @@ pub fn from_fn_attrs<'gcc, 'tcx>( #[cfg(feature = "master")] { - let inline = if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) { - InlineAttr::Never - } else if codegen_fn_attrs.inline == InlineAttr::None - && instance.def.requires_inline(cx.tcx) - { - InlineAttr::Hint - } else { - codegen_fn_attrs.inline - }; - // GCC drops `weak` from a function that is also `inline`, leaving the symbol strong, and - // the linkage is what has to survive. `inline(never)` does not conflict. - let inline = match inline { - InlineAttr::Always | InlineAttr::Hint | InlineAttr::Force { .. } - if codegen_fn_attrs.linkage.is_some_and(base::linkage_needs_weak_attribute) => - { - InlineAttr::None - } - inline => inline, - }; - if let Some(attr) = inline_attr(cx, inline, instance) { + let inline = requested_inline(cx.tcx, instance, &codegen_fn_attrs); + if let Some(attr) = inline_attr(cx, inline, instance, &codegen_fn_attrs, body, fn_abi) { if let FnAttribute::AlwaysInline = attr { func.add_attribute(FnAttribute::Inline); } diff --git a/src/callee.rs b/src/callee.rs index d3f412180da..b2bd28069d4 100644 --- a/src/callee.rs +++ b/src/callee.rs @@ -4,7 +4,7 @@ use gccjit::{Function, FunctionType}; use rustc_middle::ty::layout::{FnAbiOf, HasTyCtxt}; use rustc_middle::ty::{self, Instance, TypeVisitableExt}; -use crate::attributes; +use crate::attributes::{self, FnBody}; use crate::context::CodegenCx; /// Codegens a reference to a fn/method item, monomorphizing and @@ -70,7 +70,7 @@ pub fn get_fn<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, instance: Instance<'tcx>) cx.linkage.set(FunctionType::Extern); let func = cx.declare_fn(sym, fn_abi); - attributes::from_fn_attrs(cx, func, instance, Some(fn_abi)); + attributes::from_fn_attrs(cx, func, instance, FnBody::Declared, Some(fn_abi)); #[cfg(feature = "master")] { diff --git a/src/context.rs b/src/context.rs index 5f342e8b2dc..0f01df84f25 100644 --- a/src/context.rs +++ b/src/context.rs @@ -25,6 +25,8 @@ use rustc_target::spec::{HasTargetSpec, HasX86AbiOpt, Target, TlsModel, X86Abi}; #[cfg(feature = "master")] use crate::abi::conv_to_fn_attribute; +#[cfg(feature = "master")] +use crate::attributes::InlineAnalysis; use crate::callee::get_fn; use crate::common::SignType; use crate::type_::StructTypeKey; @@ -49,6 +51,8 @@ pub struct CodegenCx<'gcc, 'tcx> { pub functions: RefCell>>, pub intrinsics: RefCell>>, + #[cfg(feature = "master")] + pub inline_analysis: InlineAnalysis<'tcx>, pub tls_model: gccjit::TlsModel, @@ -265,6 +269,8 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { function_address_names: Default::default(), functions: RefCell::new(functions), intrinsics: RefCell::new(FxHashMap::default()), + #[cfg(feature = "master")] + inline_analysis: Default::default(), tls_model, diff --git a/src/intrinsic/mod.rs b/src/intrinsic/mod.rs index 4d2590ac81e..c50e5e9b16b 100644 --- a/src/intrinsic/mod.rs +++ b/src/intrinsic/mod.rs @@ -609,7 +609,13 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tc self.on_stack_function_params.borrow_mut().insert(func, FxHashSet::default()); - crate::attributes::from_fn_attrs(self, func, instance, None); + crate::attributes::from_fn_attrs( + self, + func, + instance, + crate::attributes::FnBody::Declared, + None, + ); func }; diff --git a/src/mono_item.rs b/src/mono_item.rs index d8170fbb085..96077657c52 100644 --- a/src/mono_item.rs +++ b/src/mono_item.rs @@ -11,6 +11,7 @@ use rustc_middle::ty::layout::{FnAbiOf, HasTypingEnv, LayoutOf}; use rustc_middle::ty::{self, Instance, TypeVisitableExt}; use rustc_span::bug; +use crate::attributes::FnBody; use crate::consts::const_alloc_type; use crate::context::CodegenCx; use crate::type_of::LayoutGccExt; @@ -188,7 +189,7 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> { self.linkage.set(base::linkage_to_gcc(linkage)); let fn_decl = self.declare_fn(symbol_name, fn_abi); - attributes::from_fn_attrs(self, fn_decl, instance, Some(fn_abi)); + attributes::from_fn_attrs(self, fn_decl, instance, FnBody::Defined, Some(fn_abi)); #[cfg(feature = "master")] if base::linkage_needs_weak_attribute(linkage) { diff --git a/tests/asm/always_inline.rs b/tests/asm/always_inline.rs new file mode 100644 index 00000000000..5352ffc657d --- /dev/null +++ b/tests/asm/always_inline.rs @@ -0,0 +1,31 @@ +//@ assembly-output: emit-asm +//@ only-x86_64 +//@ compile-flags: -Copt-level=0 + +#![crate_type = "lib"] +#![no_std] + +// At -O0 a plain `inline` hint does nothing, so every function in this chain has to keep +// always_inline. +#[inline(always)] +fn leaf(x: u32) -> u32 { + x ^ 0xa5a5 +} + +#[inline(always)] +fn mid(x: u32) -> u32 { + leaf(x).wrapping_mul(3) +} + +#[inline(always)] +fn top(x: u32) -> u32 { + mid(x).wrapping_add(7) +} + +// CHECK-LABEL: {{^"?_?}}entry{{"?}}: +// CHECK-NOT: call +// CHECK: ret +#[no_mangle] +pub fn entry(x: u32) -> u32 { + top(x) +} diff --git a/tests/asm/always_inline_address_cycles.rs b/tests/asm/always_inline_address_cycles.rs new file mode 100644 index 00000000000..4a1297c0e40 --- /dev/null +++ b/tests/asm/always_inline_address_cycles.rs @@ -0,0 +1,53 @@ +//@ assembly-output: emit-asm +//@ only-x86_64 +//@ compile-flags: -Copt-level=1 + +#![crate_type = "lib"] +#![no_std] + +// From -O1, GCC folds loads from read-only data, so calling through a static table or a vtable +// becomes a direct call, which closes the cycle. + +static TABLE: [fn(u32) -> u32; 1] = [via_table]; + +#[inline(always)] +fn via_table(n: u32) -> u32 { + table_caller(n) +} + +#[inline(always)] +fn table_caller(n: u32) -> u32 { + if n == 0 { + return 0; + } + TABLE[0](n - 1) + 1 +} + +trait Step { + fn step(&self, n: u32) -> u32; +} + +struct S; + +impl Step for S { + #[inline(always)] + fn step(&self, n: u32) -> u32 { + dyn_caller(n) + } +} + +#[inline(always)] +fn dyn_caller(n: u32) -> u32 { + if n == 0 { + return 0; + } + let step: &dyn Step = &S; + step.step(n - 1) + 1 +} + +// CHECK-LABEL: {{^"?_?}}entry{{"?}}: +// CHECK: ret +#[no_mangle] +pub fn entry(n: u32) -> u32 { + via_table(n) + dyn_caller(n) +} diff --git a/tests/asm/always_inline_cycle_caller.rs b/tests/asm/always_inline_cycle_caller.rs new file mode 100644 index 00000000000..1bd26e842e4 --- /dev/null +++ b/tests/asm/always_inline_cycle_caller.rs @@ -0,0 +1,27 @@ +//@ assembly-output: emit-asm +//@ only-x86_64 +//@ compile-flags: -Copt-level=0 + +#![crate_type = "lib"] +#![no_std] + +#[inline(always)] +fn countdown(n: u32) -> u32 { + if n == 0 { 0 } else { countdown(n - 1) } +} + +// Reaching a cycle is not being on one: only `countdown` loses always_inline. +#[inline(always)] +fn wrapper(n: u32) -> u32 { + countdown(n) +} + +// CHECK-LABEL: {{^"?_?}}entry{{"?}}: +// CHECK-NOT: {{call.*wrapper}} +// CHECK: {{call.*countdown}} +// CHECK-NOT: {{call.*wrapper}} +// CHECK: ret +#[no_mangle] +pub fn entry(n: u32) -> u32 { + wrapper(n) +} diff --git a/tests/asm/always_inline_imported_mono.rs b/tests/asm/always_inline_imported_mono.rs new file mode 100644 index 00000000000..dbbfdc3de9e --- /dev/null +++ b/tests/asm/always_inline_imported_mono.rs @@ -0,0 +1,25 @@ +//@ assembly-output: emit-asm +//@ only-x86_64 +//@ compile-flags: -Copt-level=1 +//@ aux-build: avx_generic.rs + +#![crate_type = "lib"] +#![no_std] + +// `imported::<()>` comes from the upstream crate, built with avx. We only declare it here, so its +// body must not be checked against our target features. +extern crate avx_generic; + +static TABLE: [fn(); 1] = [avx_generic::imported::<()>]; + +#[inline(always)] +fn wrapper() { + TABLE[0]() +} + +// CHECK-LABEL: {{^"?_?}}entry{{"?}}: +// CHECK: ret +#[no_mangle] +pub fn entry() { + wrapper() +} diff --git a/tests/asm/auxiliary/avx_generic.rs b/tests/asm/auxiliary/avx_generic.rs new file mode 100644 index 00000000000..7bdd841ef45 --- /dev/null +++ b/tests/asm/auxiliary/avx_generic.rs @@ -0,0 +1,21 @@ +//@ no-prefer-dynamic +//@ compile-flags: -Copt-level=1 -Ctarget-feature=+avx + +#![crate_type = "rlib"] +#![no_std] +#![feature(simd_ffi)] +#![allow(improper_ctypes)] + +use core::arch::x86_64::__m256; + +unsafe extern "C" { + fn takes_avx(value: __m256); +} + +pub fn imported() { + unsafe { takes_avx(core::mem::zeroed()) } +} + +pub fn instantiate() { + imported::<()>(); +} diff --git a/tests/compile/always_inline_eii.rs b/tests/compile/always_inline_eii.rs new file mode 100644 index 00000000000..3300fd6c4c0 --- /dev/null +++ b/tests/compile/always_inline_eii.rs @@ -0,0 +1,29 @@ +// Compiler: + +// To GCC, calling an EII declaration from its own implementation is a plain self-call. + +#![crate_type = "lib"] +#![feature(extern_item_impls)] +#![allow(incomplete_features, unused_attributes)] + +#[eii] +fn callback(n: u32) -> u32; + +#[inline(always)] +fn leaf(n: u32) -> u32 { + n - 1 +} + +#[callback] +#[inline(always)] +fn implementation(n: u32) -> u32 { + if n == 0 { + return 0; + } + callback(leaf(n)) + 1 +} + +#[unsafe(no_mangle)] +pub fn entry(n: u32) -> u32 { + implementation(n) +} diff --git a/tests/compile/always_inline_extern_decl.rs b/tests/compile/always_inline_extern_decl.rs new file mode 100644 index 00000000000..c22bea97beb --- /dev/null +++ b/tests/compile/always_inline_extern_decl.rs @@ -0,0 +1,15 @@ +// Compiler: + +// A declaration has no body to inline, so it must not get always_inline. + +#![crate_type = "lib"] +#![allow(unused_attributes)] + +unsafe extern "C" { + #[inline(always)] + fn abs(x: i32) -> i32; +} + +pub fn entry(x: i32) -> i32 { + unsafe { abs(x) } +} diff --git a/tests/compile/always_inline_symbol_alias.rs b/tests/compile/always_inline_symbol_alias.rs new file mode 100644 index 00000000000..ac402c3035e --- /dev/null +++ b/tests/compile/always_inline_symbol_alias.rs @@ -0,0 +1,29 @@ +// Compiler: + +// `alias` is `recurse` under another name, so the Rust call graph can't see this recursion. + +#![crate_type = "lib"] +#![allow(unused_attributes)] + +unsafe extern "C" { + #[link_name = "recurse"] + fn alias(n: u32) -> u32; +} + +#[inline(always)] +fn leaf(n: u32) -> u32 { + n - 1 +} + +#[unsafe(no_mangle)] +#[inline(always)] +pub extern "C" fn recurse(n: u32) -> u32 { + if n == 0 { + return 0; + } + unsafe { alias(leaf(n)) + 1 } +} + +pub fn entry(n: u32) -> u32 { + recurse(n) +} diff --git a/tests/compile/always_inline_variadic.rs b/tests/compile/always_inline_variadic.rs new file mode 100644 index 00000000000..7c262e24e91 --- /dev/null +++ b/tests/compile/always_inline_variadic.rs @@ -0,0 +1,19 @@ +// Compiler: + +// GCC won't inline a function that uses va_arg, cycle or not. + +#![crate_type = "lib"] + +#[inline(always)] +fn helper(n: u32) -> u32 { + n + 1 +} + +#[inline(always)] +pub unsafe extern "C" fn variadic(n: u32, mut args: ...) -> u32 { + helper(n) + unsafe { args.next_arg::() } +} + +pub fn entry(n: u32) -> u32 { + unsafe { variadic(n, n) } +} diff --git a/tests/compile/always_inline_x86_interrupt.rs b/tests/compile/always_inline_x86_interrupt.rs new file mode 100644 index 00000000000..d3f3ead5006 --- /dev/null +++ b/tests/compile/always_inline_x86_interrupt.rs @@ -0,0 +1,27 @@ +// Compiler: + +// Interrupt handlers are built with general-regs-only, and GCC won't inline normally-built code +// into them. + +#![feature(abi_x86_interrupt)] +#![crate_type = "lib"] + +static mut STATE: u64 = 0; + +#[inline(always)] +fn leaf(x: u64) -> u64 { + x.wrapping_add(1) +} + +#[inline(always)] +fn wrapper(x: u64) -> u64 { + leaf(x) +} + +pub extern "x86-interrupt" fn via_wrapper(_frame: u64) { + unsafe { STATE = wrapper(STATE) } +} + +pub extern "x86-interrupt" fn direct(_frame: u64) { + unsafe { STATE = leaf(STATE) } +} diff --git a/tests/lang_tests.rs b/tests/lang_tests.rs index 7ec0ab877b0..c149518c12d 100644 --- a/tests/lang_tests.rs +++ b/tests/lang_tests.rs @@ -286,6 +286,9 @@ fn compile_tests(tempdir: PathBuf, c_objects_dir: PathBuf, current_dir: String) "global_asm_nul_byte.rs", "naked_asm_nul_byte.rs", "x86_interrupt_first_arg_byval.rs", + "always_inline_x86_interrupt.rs", + // C-variadic definitions are still unstable on m68k. + "always_inline_variadic.rs", ], ); } diff --git a/tests/run/always_inline.rs b/tests/run/always_inline.rs index ebd741ee090..22de6b25c43 100644 --- a/tests/run/always_inline.rs +++ b/tests/run/always_inline.rs @@ -3,7 +3,7 @@ // Run-time: // status: 0 -#![feature(no_core)] +#![feature(no_core, stmt_expr_attributes)] #![no_std] #![no_core] #![no_main] @@ -44,10 +44,74 @@ fn fib_a(n: u8) -> u8 { fib_b(n - 1) + fib_b(n - 2) } +// These cycles only show up after monomorphization: the generic callers call +// `::step` and `::call_mut`, not the impls. +trait Step { + fn step(n: u8) -> u8; +} + +struct S; + +impl Step for S { + #[inline(always)] + fn step(n: u8) -> u8 { + if n == 0 { + return 0; + } + drive::(n - 1) + 1 + } +} + +#[inline(always)] +fn drive(n: u8) -> u8 { + T::step(n) +} + +#[inline(always)] +fn apply u8>(mut f: F, n: u8) -> u8 { + f(n) +} + +#[inline(always)] +fn countdown(n: u8) -> u8 { + if n == 0 { + return 0; + } + apply( + #[inline(always)] + |m| countdown(m), + n - 1, + ) + 1 +} + +// GCC turns a call through a known fn pointer into a direct call, which closes the cycle. +#[inline(always)] +fn via_pointer(n: u8) -> u8 { + if n == 0 { + return 0; + } + let f: fn(u8) -> u8 = via_direct; + f(n - 1) + 1 +} + +#[inline(always)] +fn via_direct(n: u8) -> u8 { + via_pointer(n) +} + #[no_mangle] extern "C" fn main(argc: i32, _argv: *const *const u8) -> i32 { if fib(2) != fib_a(2) { intrinsics::abort(); } + if drive::(3) != 3 { + intrinsics::abort(); + } + if countdown(3) != 3 { + intrinsics::abort(); + } + if via_direct(3) != 3 { + intrinsics::abort(); + } 0 } From 0e658471cf12a958daa8c35e44069313081fcd8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Bernier?= Date: Wed, 23 Sep 2026 00:49:44 -0400 Subject: [PATCH 2/3] ci: refresh the apt index before installing librsvg dependencies The runner image ships a package index naming glib2.0 2.80.0-6ubuntu3.8, which the Ubuntu mirror no longer serves, so all four --projects jobs of #976 failed in this step with 404s before building anything. --- .github/workflows/ci.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f5cc409e36..a394748abd8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -56,7 +56,10 @@ jobs: - name: Install the libraries needed to build librsvg if: ${{ contains(matrix.commands, '--projects') }} - run: sudo apt-get install libcairo2-dev libpango1.0-dev libfontconfig1-dev libfreetype-dev libharfbuzz-dev libxml2-dev libglib2.0-dev + # The runner image's package index can be stale. + run: | + sudo apt-get update + sudo apt-get install libcairo2-dev libpango1.0-dev libfontconfig1-dev libfreetype-dev libharfbuzz-dev libxml2-dev libglib2.0-dev - name: Download artifact run: curl -LO https://github.com/rust-lang/gcc/releases/latest/download/${{ matrix.libgccjit_version.gcc }} From 4db4e227e3f53c19f645bac357f70bf657405076 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Bernier?= Date: Wed, 23 Sep 2026 09:22:22 -0400 Subject: [PATCH 3/3] Only demote always-inline functions on actual cycles The previous commit demoted some functions without looking for a cycle. This one adds the missing edges instead, so these functions keep always_inline unless they are really on a cycle: - EII implementations, and exported functions that some extern declaration imports. The collector skips foreign declarations, so we now scan each body for calls, fn-pointer casts and constants that refer to one, and map the declaration's symbol to the definition in this codegen unit (including the EII wrapper symbol). A declaration defined in another unit is left alone: GCC can't inline across units. - Recursion through a plain helper when optimizing. The same scan tells direct calls apart from taken addresses: reified fn pointers, closures turned into fn pointers, and fn pointers in constants. The collector lists each function once, so one that is both called and has its address taken counts as an address. A body that coerces something to dyn Trait counts every function it uses as an address, since we don't work out what the new vtable holds. GCC is fine with a cycle through a plain function made only of direct calls; it only rejects cycles made purely of always_inline functions, and addresses that resolve back into a function they were inlined into. So there are now two checks: SCCs over always-inline functions only, and, when optimizing, an address edge whose ends share an SCC in the full graph. #[rustc_force_inline] now goes through the same cycle check: the MIR inliner only rejects call cycles, not ones through addresses or foreign declarations. A regex release build takes the same time (41.9-42.4 s on both). Tests: - tests/asm/always_inline_foreign_decls.rs: an EII implementation and an imported exported function keep always_inline at -O0. - tests/asm/always_inline_helper_recursion.rs: recursing through a plain helper at -O2 keeps it. - tests/asm/always_inline_call_and_address.rs: a helper that both calls a forced function and returns its address. It crashes GCC at -O2 if the address check is missing or the call hides the address. The first two fail on the previous commit. --- src/attributes.rs | 379 ++++++++++++++------ tests/asm/always_inline_call_and_address.rs | 31 ++ tests/asm/always_inline_foreign_decls.rs | 43 +++ tests/asm/always_inline_helper_recursion.rs | 34 ++ 4 files changed, 386 insertions(+), 101 deletions(-) create mode 100644 tests/asm/always_inline_call_and_address.rs create mode 100644 tests/asm/always_inline_foreign_decls.rs create mode 100644 tests/asm/always_inline_helper_recursion.rs diff --git a/src/attributes.rs b/src/attributes.rs index d8002e14146..9a6868ca80e 100644 --- a/src/attributes.rs +++ b/src/attributes.rs @@ -1,7 +1,5 @@ #[cfg(feature = "master")] use std::cell::{OnceCell, RefCell}; -#[cfg(feature = "master")] -use std::iter; #[cfg(feature = "master")] use gccjit::FnAttribute; @@ -18,20 +16,24 @@ use rustc_data_structures::graph::vec_graph::VecGraph; use rustc_hir::attrs::InlineAttr; use rustc_hir::attrs::InstructionSetAttr; #[cfg(feature = "master")] -use rustc_hir::def::DefKind; -#[cfg(feature = "master")] use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; #[cfg(feature = "master")] -use rustc_middle::mir::interpret::{AllocId, GlobalAlloc}; +use rustc_middle::mir::interpret::{AllocId, GlobalAlloc, Scalar}; +#[cfg(feature = "master")] +use rustc_middle::mir::visit::Visitor; +#[cfg(feature = "master")] +use rustc_middle::mir::{self, Location, TerminatorKind, traversal}; #[cfg(feature = "master")] use rustc_middle::mono::{CollectionMode, MonoItem}; use rustc_middle::ty; #[cfg(feature = "master")] +use rustc_middle::ty::adjustment::PointerCoercion; +#[cfg(feature = "master")] use rustc_middle::ty::layout::FnAbiOf; #[cfg(feature = "master")] use rustc_session::config::OptLevel; #[cfg(feature = "master")] -use rustc_span::def_id::{DefId, LOCAL_CRATE}; +use rustc_span::def_id::DefId; use rustc_target::callconv::FnAbi; #[cfg(feature = "master")] use rustc_target::spec::Arch; @@ -55,7 +57,16 @@ pub struct InlineAnalysis<'tcx> { /// Whether GCC could end up inlining the instance into itself. in_cycle: RefCell, bool>>, interrupt_callees: OnceCell>>, - foreign_imports: OnceCell>, + definitions: OnceCell>>, +} + +/// How a function reaches something GCC could inline into it. +#[cfg(feature = "master")] +#[derive(Clone, Copy, PartialEq, Eq)] +enum Edge { + Call, + /// Only its address is taken; GCC may still turn that into a direct call. + Address, } /// The inlining we ask GCC for, before checking whether it can honor `always_inline`. @@ -84,6 +95,14 @@ fn requested_inline<'tcx>( } } +#[cfg(feature = "master")] +fn requests_forced_inline<'tcx>(tcx: ty::TyCtxt<'tcx>, instance: ty::Instance<'tcx>) -> bool { + matches!( + requested_inline(tcx, instance, &tcx.codegen_instance_attrs(instance.def)), + InlineAttr::Always | InlineAttr::Force { .. } + ) +} + /// Whether GCC may inline `instance` into a caller. At `-O0` it only inlines `always_inline` /// functions; above that, anything not marked `noinline`. #[cfg(feature = "master")] @@ -99,15 +118,38 @@ fn gcc_may_inline<'tcx>( } } -/// Functions whose code GCC could pull into `instance`: its callees, plus anything whose address -/// it takes (fn pointers, vtables, statics), since GCC turns calls through a known address into -/// direct calls and inlines those too. +/// Functions defined in this unit, under every symbol GCC knows them by. A foreign declaration +/// naming one of these symbols is, as far as GCC is concerned, that function. #[cfg(feature = "master")] -fn inline_edges<'tcx>( - tcx: ty::TyCtxt<'tcx>, +fn definitions<'a, 'gcc, 'tcx>( + cx: &'a CodegenCx<'gcc, 'tcx>, +) -> &'a FxHashMap<&'tcx str, ty::Instance<'tcx>> { + cx.inline_analysis.definitions.get_or_init(|| { + let tcx = cx.tcx; + let mut definitions = FxHashMap::default(); + for item in cx.codegen_unit.items().keys() { + let MonoItem::Fn(instance) = *item else { continue }; + definitions.insert(tcx.symbol_name(instance).name, instance); + // An EII implementation also gets a wrapper under the declaration's symbol, and + // `add_function_aliases` annotates it with this same instance. + let attrs = tcx.codegen_instance_attrs(instance.def); + for &(alias, ..) in &attrs.foreign_item_symbol_aliases { + definitions.insert(tcx.symbol_name(ty::Instance::mono(tcx, alias)).name, instance); + } + } + definitions + }) +} + +/// Functions whose code GCC could pull into `instance`: what it calls, and what it takes the +/// address of (fn pointers, vtables, statics), since GCC turns calls through a known address into +/// direct calls. +#[cfg(feature = "master")] +fn inline_edges<'gcc, 'tcx>( + cx: &CodegenCx<'gcc, 'tcx>, instance: ty::Instance<'tcx>, - optimize: OptLevel, -) -> Vec> { +) -> Vec<(ty::Instance<'tcx>, Edge)> { + let tcx = cx.tcx; // Nothing to inline without a body. Monomorphizations we import from another crate count as // bodyless: querying them would also check them against the wrong target features. let has_body = match instance.def { @@ -117,57 +159,197 @@ fn inline_edges<'tcx>( | ty::InstanceKind::Virtual(..) => false, ty::InstanceKind::Shim(_) => true, }; - let mut edges = Vec::new(); if !has_body || !tcx.should_codegen_locally(instance) { - return edges; + return Vec::new(); } // The collector already resolved all of this, and reported its errors. let Ok((used, _)) = tcx.items_of_instance((instance, CollectionMode::UsedItems)) else { - return edges; + return Vec::new(); + }; + // The collector doesn't say which uses are calls, and skips foreign declarations. + let body = tcx.instance_mir(instance.def); + let mut refs = BodyRefs { + tcx, + instance, + body, + definitions: definitions(cx), + calls: FxHashSet::default(), + addresses: FxHashSet::default(), + makes_vtables: false, }; + for (block, data) in traversal::mono_reachable(body, tcx, instance) { + refs.visit_basic_block_data(block, data); + } for item in used { match item.node { - MonoItem::Fn(callee) => edges.push(callee), - MonoItem::Static(def_id) => static_fn_addresses(tcx, def_id, &mut edges), + MonoItem::Fn(callee) => { + if !refs.calls.contains(&callee) { + refs.addresses.insert(callee); + } + } + MonoItem::Static(def_id) => { + let mut pending = Vec::new(); + static_initializer(tcx, def_id, &mut pending); + for function in allocated_fns(tcx, pending) { + refs.address_of(function); + } + } MonoItem::GlobalAsm(_) => {} } } - edges.retain(|&callee| gcc_may_inline(tcx, callee, optimize)); - edges + // We don't work out which methods a new vtable holds, so with one around, anything might be + // in it. + if refs.makes_vtables { + refs.addresses.extend(refs.calls.drain()); + } + + // A function that is both called and has its address taken counts as the latter. + let calls = refs.calls.iter().filter(|&callee| !refs.addresses.contains(callee)); + let edges = calls + .map(|&callee| (callee, Edge::Call)) + .chain(refs.addresses.iter().map(|&callee| (callee, Edge::Address))); + let optimize = cx.sess().opts.optimize; + edges.filter(|&(callee, _)| gcc_may_inline(tcx, callee, optimize)).collect() } -/// Functions reachable through the initializer of `def_id`. GCC can fold loads from read-only -/// data, so a call through a static fn table can become a direct call. +/// What a body does that the collector doesn't tell us: which functions it calls directly and +/// which it takes the address of. Foreign declarations are mapped to what they name. #[cfg(feature = "master")] -fn static_fn_addresses<'tcx>( +struct BodyRefs<'a, 'tcx> { tcx: ty::TyCtxt<'tcx>, - def_id: DefId, - out: &mut Vec>, -) { - fn initializer_ptrs(tcx: ty::TyCtxt<'_>, def_id: DefId, pending: &mut Vec) { - if tcx.is_foreign_item(def_id) - || !tcx.should_codegen_locally(ty::Instance::mono(tcx, def_id)) + instance: ty::Instance<'tcx>, + body: &'tcx mir::Body<'tcx>, + definitions: &'a FxHashMap<&'tcx str, ty::Instance<'tcx>>, + calls: FxHashSet>, + addresses: FxHashSet>, + /// Whether the body coerces something to `dyn Trait`, creating a vtable. + makes_vtables: bool, +} + +#[cfg(feature = "master")] +impl<'a, 'tcx> BodyRefs<'a, 'tcx> { + fn monomorphize>>(&self, value: T) -> T { + self.instance.instantiate_mir_and_normalize_erasing_regions( + self.tcx, + ty::TypingEnv::fully_monomorphized(), + ty::EarlyBinder::bind(self.tcx, value), + ) + } + + fn resolve(&self, fn_ty: ty::Ty<'tcx>) -> Option> { + let ty::FnDef(def_id, args) = *self.monomorphize(fn_ty).kind() else { return None }; + let typing_env = ty::TypingEnv::fully_monomorphized(); + ty::Instance::try_resolve(self.tcx, typing_env, def_id, args.no_bound_vars()?).ok()? + } + + /// The function GCC will see for `function`: the definition a foreign declaration names, or + /// nothing if that isn't in this unit or we don't generate the function here. + fn in_this_unit(&self, function: ty::Instance<'tcx>) -> Option> { + match function.def { + ty::InstanceKind::Item(def_id) if self.tcx.is_foreign_item(def_id) => { + let symbol = self.tcx.symbol_name(ty::Instance::mono(self.tcx, def_id)).name; + self.definitions.get(symbol).copied() + } + _ => self.tcx.should_codegen_locally(function).then_some(function), + } + } + + fn address_of(&mut self, function: ty::Instance<'tcx>) { + if let Some(function) = self.in_this_unit(function) { + self.addresses.insert(function); + } + } +} + +#[cfg(feature = "master")] +impl<'a, 'tcx> Visitor<'tcx> for BodyRefs<'a, 'tcx> { + fn visit_terminator(&mut self, terminator: &mir::Terminator<'tcx>, location: Location) { + if let TerminatorKind::Call { ref func, .. } | TerminatorKind::TailCall { ref func, .. } = + terminator.kind + && let Some(callee) = self.resolve(func.ty(self.body, self.tcx)) + && let Some(callee) = self.in_this_unit(callee) { - return; + self.calls.insert(callee); } - if let Ok(alloc) = tcx.eval_static_initializer(def_id) { - pending.extend(alloc.inner().provenance().ptrs().values().map(|prov| prov.alloc_id())); + self.super_terminator(terminator, location); + } + + fn visit_rvalue(&mut self, rvalue: &mir::Rvalue<'tcx>, location: Location) { + let typing_env = ty::TypingEnv::fully_monomorphized(); + if let mir::Rvalue::Cast(mir::CastKind::PointerCoercion(coercion, _), ref operand, target) = + *rvalue + { + let source = self.monomorphize(operand.ty(self.body, self.tcx)); + match (coercion, *source.kind()) { + (PointerCoercion::ReifyFnPointer(_), ty::FnDef(def_id, args)) => { + if let Some(args) = args.no_bound_vars() + && let Some(function) = + ty::Instance::resolve_for_fn_ptr(self.tcx, typing_env, def_id, args) + { + self.address_of(function); + } + } + (PointerCoercion::ClosureFnPointer(_), ty::Closure(def_id, args)) => { + let kind = ty::ClosureKind::FnOnce; + self.address_of(ty::Instance::resolve_closure(self.tcx, def_id, args, kind)); + } + (PointerCoercion::Unsize, _) => { + let target = self.monomorphize(target); + self.makes_vtables |= target.walk().any(|arg| { + arg.as_type().is_some_and(|ty| matches!(ty.kind(), ty::Dynamic(..))) + }); + } + _ => {} + } } + self.super_rvalue(rvalue, location); } - let mut pending = Vec::new(); - initializer_ptrs(tcx, def_id, &mut pending); + fn visit_const_operand(&mut self, constant: &mir::ConstOperand<'tcx>, _: Location) { + let typing_env = ty::TypingEnv::fully_monomorphized(); + let Ok(value) = + self.monomorphize(constant.const_).eval(self.tcx, typing_env, constant.span) + else { + return; + }; + let root = match value { + mir::ConstValue::Scalar(Scalar::Ptr(ptr, _)) => ptr.provenance.alloc_id(), + mir::ConstValue::Indirect { alloc_id, .. } + | mir::ConstValue::Slice { alloc_id, .. } => alloc_id, + mir::ConstValue::Scalar(Scalar::Int(_)) | mir::ConstValue::ZeroSized => return, + }; + for function in allocated_fns(self.tcx, vec![root]) { + self.address_of(function); + } + } +} + +#[cfg(feature = "master")] +fn static_initializer(tcx: ty::TyCtxt<'_>, def_id: DefId, pending: &mut Vec) { + if tcx.is_foreign_item(def_id) || !tcx.should_codegen_locally(ty::Instance::mono(tcx, def_id)) { + return; + } + if let Ok(alloc) = tcx.eval_static_initializer(def_id) { + pending.extend(alloc.inner().provenance().ptrs().values().map(|prov| prov.alloc_id())); + } +} + +/// Functions whose addresses are stored in these allocations, or in anything they point to. +/// GCC can fold loads from read-only data, so a call through a static fn table can become a +/// direct call. +#[cfg(feature = "master")] +fn allocated_fns<'tcx>( + tcx: ty::TyCtxt<'tcx>, + mut pending: Vec, +) -> Vec> { + let mut functions = Vec::new(); let mut seen = FxHashSet::default(); while let Some(alloc_id) = pending.pop() { if !seen.insert(alloc_id) { continue; } match tcx.global_alloc(alloc_id) { - GlobalAlloc::Function { instance, .. } => { - if tcx.should_codegen_locally(instance) { - out.push(instance); - } - } + GlobalAlloc::Function { instance, .. } => functions.push(instance), GlobalAlloc::Memory(alloc) => pending .extend(alloc.inner().provenance().ptrs().values().map(|prov| prov.alloc_id())), GlobalAlloc::VTable(ty, dyn_ty) => pending.push( @@ -178,17 +360,17 @@ fn static_fn_addresses<'tcx>( .map(|principal| tcx.instantiate_bound_regions_with_erased(principal)), )), ), - GlobalAlloc::Static(nested) => initializer_ptrs(tcx, nested, &mut pending), + GlobalAlloc::Static(nested) => static_initializer(tcx, nested, &mut pending), GlobalAlloc::TypeId { .. } => {} } } + functions } /// Whether GCC could end up inlining `root` into itself. /// /// Classifies everything reachable from `root` at once. Only functions on a cycle are affected: -/// once they lose `always_inline`, callers that merely reach the cycle can keep it. When -/// optimizing, cycles through plain helpers count too, since GCC may inline those as well. +/// once they lose `always_inline`, callers that merely reach the cycle can keep it. #[cfg(feature = "master")] fn in_inline_cycle<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, root: ty::Instance<'tcx>) -> bool { if let Some(&in_cycle) = cx.inline_analysis.in_cycle.borrow().get(&root) { @@ -197,7 +379,6 @@ fn in_inline_cycle<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, root: ty::Instance<'t // Collect the part of the graph reachable from `root` that hasn't been classified yet. // Classified functions can be left out: their SCC is complete, so nothing new can join it. - let optimize = cx.sess().opts.optimize; let mut nodes = vec![root]; let mut index = FxHashMap::from_iter([(root, 0)]); let mut edges = Vec::new(); @@ -205,7 +386,7 @@ fn in_inline_cycle<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, root: ty::Instance<'t let classified = cx.inline_analysis.in_cycle.borrow(); let mut caller = 0; while let Some(&instance) = nodes.get(caller) { - for callee in inline_edges(cx.tcx, instance, optimize) { + for (callee, edge) in inline_edges(cx, instance) { if classified.contains_key(&callee) { continue; } @@ -213,27 +394,60 @@ fn in_inline_cycle<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, root: ty::Instance<'t nodes.push(callee); nodes.len() - 1 }); - edges.push((caller, callee)); + edges.push((caller, callee, edge)); } caller += 1; } } + let forced: Vec = + nodes.iter().map(|&node| requests_forced_inline(cx.tcx, node)).collect(); - let mut calls_itself = vec![false; nodes.len()]; - for &(caller, callee) in &edges { - calls_itself[caller] |= caller == callee; - } - let sccs: Sccs = Sccs::new(&VecGraph::::new(nodes.len(), edges)); - let mut scc_sizes = vec![0usize; sccs.num_sccs()]; - for node in 0..nodes.len() { - scc_sizes[sccs.scc(node)] += 1; + // GCC rejects any cycle made only of `always_inline` functions. A plain function in the middle + // breaks it up as long as everything is a direct call: GCC inlines those early and never + // tries to inline the plain function into itself. + let forced_edges = edges + .iter() + .filter(|&&(from, to, _)| forced[from] && forced[to]) + .map(|&(from, to, _)| (from, to)); + let mut in_cycle = in_nontrivial_scc(nodes.len(), forced_edges.collect()); + + // When optimizing, GCC also inlines plain functions, and can then resolve an address taken in + // one of them into a direct call. If that call leads back to a function it was inlined into, + // GCC rejects it, so the function whose address is taken loses `always_inline`. + if cx.sess().opts.optimize != OptLevel::No { + let all = Sccs::::new(&VecGraph::::new( + nodes.len(), + edges.iter().map(|&(from, to, _)| (from, to)).collect(), + )); + for &(from, to, edge) in &edges { + if edge == Edge::Address && forced[to] && all.scc(from) == all.scc(to) { + in_cycle[to] = true; + } + } } - let mut in_cycle = cx.inline_analysis.in_cycle.borrow_mut(); - for (node, &instance) in nodes.iter().enumerate() { - in_cycle.insert(instance, calls_itself[node] || scc_sizes[sccs.scc(node)] > 1); + let mut classified = cx.inline_analysis.in_cycle.borrow_mut(); + classified.extend(nodes.iter().copied().zip(in_cycle.iter().copied())); + in_cycle[0] +} + +/// Which nodes lie on a cycle: a strongly connected component with more than one node, or a +/// node with an edge to itself. +#[cfg(feature = "master")] +fn in_nontrivial_scc(num_nodes: usize, edges: Vec<(usize, usize)>) -> Vec { + let mut in_cycle = vec![false; num_nodes]; + for &(from, to) in &edges { + in_cycle[from] |= from == to; } - in_cycle[&root] + let sccs = Sccs::::new(&VecGraph::::new(num_nodes, edges)); + let mut sizes = vec![0usize; sccs.num_sccs()]; + for node in 0..num_nodes { + sizes[sccs.scc(node)] += 1; + } + for (node, in_cycle) in in_cycle.iter_mut().enumerate() { + *in_cycle |= sizes[sccs.scc(node)] > 1; + } + in_cycle } /// `always_inline` functions that an `x86-interrupt` handler in this unit could inline. Handlers @@ -243,7 +457,6 @@ fn interrupt_callees<'a, 'gcc, 'tcx>( cx: &'a CodegenCx<'gcc, 'tcx>, ) -> &'a FxHashSet> { cx.inline_analysis.interrupt_callees.get_or_init(|| { - let optimize = cx.sess().opts.optimize; cx.codegen_unit .items() .keys() @@ -254,46 +467,13 @@ fn interrupt_callees<'a, 'gcc, 'tcx>( .filter(|&instance| { is_x86_interrupt(Some(cx.fn_abi_of_instance(instance, ty::List::empty()))) }) - .flat_map(|handler| inline_edges(cx.tcx, handler, optimize)) - .filter(|&callee| { - matches!( - requested_inline(cx.tcx, callee, &cx.tcx.codegen_instance_attrs(callee.def)), - InlineAttr::Always | InlineAttr::Force { .. } - ) - }) + .flat_map(|handler| inline_edges(cx, handler)) + .map(|(callee, _)| callee) + .filter(|&callee| requests_forced_inline(cx.tcx, callee)) .collect() }) } -/// Whether some foreign declaration (`extern` block item or EII declaration) can call `instance`. -/// GCC sees such a call as a direct call to the definition; the Rust call graph doesn't see it. -#[cfg(feature = "master")] -fn called_through_foreign_decl<'gcc, 'tcx>( - cx: &CodegenCx<'gcc, 'tcx>, - instance: ty::Instance<'tcx>, - attrs: &CodegenFnAttrs, -) -> bool { - // An EII implementation is always reachable from its declaration, via the forwarding - // wrapper that `add_function_aliases` annotates with this same instance. - if !attrs.foreign_item_symbol_aliases.is_empty() { - return true; - } - if !attrs.contains_extern_indicator() { - return false; - } - let tcx = cx.tcx; - let foreign_imports = cx.inline_analysis.foreign_imports.get_or_init(|| { - iter::once(LOCAL_CRATE) - .chain(tcx.crates(()).iter().copied()) - .flat_map(|krate| tcx.foreign_modules(krate).values()) - .flat_map(|module| &module.foreign_items) - .filter(|&&def_id| tcx.def_kind(def_id) == DefKind::Fn) - .map(|&def_id| tcx.symbol_name(ty::Instance::mono(tcx, def_id)).name) - .collect() - }); - foreign_imports.contains(tcx.symbol_name(instance).name) -} - /// Get GCC attribute for the provided inline heuristic, attached to `instance`. #[cfg(feature = "master")] #[inline] @@ -301,7 +481,6 @@ fn inline_attr<'gcc, 'tcx>( cx: &CodegenCx<'gcc, 'tcx>, inline: InlineAttr, instance: ty::Instance<'tcx>, - attrs: &CodegenFnAttrs, body: FnBody, fn_abi: Option<&FnAbi<'tcx, ty::Ty<'tcx>>>, ) -> Option> { @@ -313,10 +492,8 @@ fn inline_attr<'gcc, 'tcx>( // GCC never inlines functions that use va_arg. && !fn_abi.is_some_and(|fn_abi| fn_abi.c_variadic) && !interrupt_callees(cx).contains(&instance) - && !called_through_foreign_decl(cx, instance, attrs) - // The MIR inliner already rejects `#[rustc_force_inline]` cycles. - && (matches!(inline, InlineAttr::Force { .. }) || !in_inline_cycle(cx, instance)); - if can_inline { Some(FnAttribute::AlwaysInline) } else { Some(FnAttribute::Inline) } + && !in_inline_cycle(cx, instance); + Some(if can_inline { FnAttribute::AlwaysInline } else { FnAttribute::Inline }) } InlineAttr::Hint => Some(FnAttribute::Inline), InlineAttr::Never => { @@ -354,7 +531,7 @@ pub fn from_fn_attrs<'gcc, 'tcx>( #[cfg(feature = "master")] { let inline = requested_inline(cx.tcx, instance, &codegen_fn_attrs); - if let Some(attr) = inline_attr(cx, inline, instance, &codegen_fn_attrs, body, fn_abi) { + if let Some(attr) = inline_attr(cx, inline, instance, body, fn_abi) { if let FnAttribute::AlwaysInline = attr { func.add_attribute(FnAttribute::Inline); } diff --git a/tests/asm/always_inline_call_and_address.rs b/tests/asm/always_inline_call_and_address.rs new file mode 100644 index 00000000000..9957b8c71bc --- /dev/null +++ b/tests/asm/always_inline_call_and_address.rs @@ -0,0 +1,31 @@ +//@ assembly-output: emit-asm +//@ only-x86_64 +//@ compile-flags: -Copt-level=2 + +#![crate_type = "lib"] +#![no_std] + +// `helper` both calls `forced` and returns its address. Once GCC inlines `helper`, that address +// turns into direct calls back into `forced`, so `forced` must lose always_inline. + +#[inline(always)] +fn forced(n: u32) -> u32 { + if n == 0 { + return 0; + } + let f = helper(); + f(n - 1).wrapping_add(f(n / 2)).wrapping_add(n) +} + +#[inline] +fn helper() -> fn(u32) -> u32 { + let _ = forced(0); + forced +} + +// CHECK-LABEL: {{^"?_?}}entry{{"?}}: +// CHECK: .size +#[no_mangle] +pub fn entry(n: u32) -> u32 { + forced(n) +} diff --git a/tests/asm/always_inline_foreign_decls.rs b/tests/asm/always_inline_foreign_decls.rs new file mode 100644 index 00000000000..5ef1cb94669 --- /dev/null +++ b/tests/asm/always_inline_foreign_decls.rs @@ -0,0 +1,43 @@ +//@ assembly-output: emit-asm +//@ only-x86_64 +//@ compile-flags: -Copt-level=0 + +#![crate_type = "lib"] +#![no_std] +#![feature(extern_item_impls)] +#![allow(incomplete_features, unused_attributes)] + +// Being reachable from a foreign declaration only matters when it closes a cycle. + +#[eii] +fn hook(n: u32) -> u32; + +#[hook] +#[inline(always)] +fn hook_impl(n: u32) -> u32 { + n.wrapping_mul(3) +} + +unsafe extern "C" { + #[link_name = "exported"] + fn exported_alias(n: u32) -> u32; +} + +#[no_mangle] +#[inline(always)] +pub extern "C" fn exported(n: u32) -> u32 { + n.wrapping_add(5) +} + +#[no_mangle] +pub fn through_decls(n: u32) -> u32 { + hook(n).wrapping_add(unsafe { exported_alias(n) }) +} + +// CHECK-LABEL: {{^"?_?}}entry{{"?}}: +// CHECK-NOT: call +// CHECK: ret +#[no_mangle] +pub fn entry(n: u32) -> u32 { + hook_impl(n).wrapping_add(exported(n)) +} diff --git a/tests/asm/always_inline_helper_recursion.rs b/tests/asm/always_inline_helper_recursion.rs new file mode 100644 index 00000000000..91cdbe42bfd --- /dev/null +++ b/tests/asm/always_inline_helper_recursion.rs @@ -0,0 +1,34 @@ +//@ assembly-output: emit-asm +//@ only-x86_64 +//@ compile-flags: -Copt-level=2 + +#![crate_type = "lib"] +#![no_std] + +// Recursing through a plain helper is fine for GCC as long as every step is a direct call, so +// `step` keeps always_inline. +#[inline(always)] +fn step(n: u32) -> u32 { + if n == 0 { + return 0; + } + let acc = n.wrapping_mul(0x9e3779b1).rotate_left(5); + helper(n - 1).wrapping_add(helper(n / 2)).wrapping_add(acc) +} + +fn helper(n: u32) -> u32 { + step(n) +} + +#[no_mangle] +pub fn other(n: u32) -> u32 { + step(n) +} + +// CHECK-LABEL: {{^"?_?}}entry{{"?}}: +// CHECK-NOT: {{(call|jmp).*step}} +// CHECK: .size +#[no_mangle] +pub fn entry(n: u32) -> u32 { + step(n.wrapping_add(1)) +}