Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically enable cross-crate inlining for small functions #116505

Merged
merged 2 commits into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,7 @@ fn test_unstable_options_tracking_hash() {
);
tracked!(codegen_backend, Some("abc".to_string()));
tracked!(crate_attr, vec!["abc".to_string()]);
tracked!(cross_crate_inline_threshold, Some(200));
tracked!(debug_info_for_profiling, true);
tracked!(debug_macros, true);
tracked!(dep_info_omit_d_target, true);
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1273,6 +1273,10 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
self.root.tables.optimized_mir.get(self, id).is_some()
}

fn cross_crate_inlinable(self, id: DefIndex) -> bool {
self.root.tables.cross_crate_inlinable.get(self, id).unwrap_or(false)
}

fn get_fn_has_self_parameter(self, id: DefIndex, sess: &'a Session) -> bool {
self.root
.tables
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ provide! { tcx, def_id, other, cdata,
item_attrs => { tcx.arena.alloc_from_iter(cdata.get_item_attrs(def_id.index, tcx.sess)) }
is_mir_available => { cdata.is_item_mir_available(def_id.index) }
is_ctfe_mir_available => { cdata.is_ctfe_mir_available(def_id.index) }
cross_crate_inlinable => { cdata.cross_crate_inlinable(def_id.index) }

dylib_dependency_formats => { cdata.get_dylib_dependency_formats(tcx) }
is_private_dep => {
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,7 @@ fn should_encode_mir(
|| (tcx.sess.opts.output_types.should_codegen()
&& reachable_set.contains(&def_id)
&& (generics.requires_monomorphization(tcx)
|| tcx.codegen_fn_attrs(def_id).requests_inline()));
|| tcx.cross_crate_inlinable(def_id)));
// The function has a `const` modifier or is in a `#[const_trait]`.
let is_const_fn = tcx.is_const_fn_raw(def_id.to_def_id())
|| tcx.is_const_default_method(def_id.to_def_id());
Expand Down Expand Up @@ -1615,6 +1615,9 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
debug!("EntryBuilder::encode_mir({:?})", def_id);
if encode_opt {
record!(self.tables.optimized_mir[def_id.to_def_id()] <- tcx.optimized_mir(def_id));
self.tables
.cross_crate_inlinable
.set(def_id.to_def_id().index, Some(self.tcx.cross_crate_inlinable(def_id)));
record!(self.tables.closure_saved_names_of_captured_variables[def_id.to_def_id()]
<- tcx.closure_saved_names_of_captured_variables(def_id));

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,7 @@ define_tables! {
object_lifetime_default: Table<DefIndex, LazyValue<ObjectLifetimeDefault>>,
optimized_mir: Table<DefIndex, LazyValue<mir::Body<'static>>>,
mir_for_ctfe: Table<DefIndex, LazyValue<mir::Body<'static>>>,
cross_crate_inlinable: Table<DefIndex, bool>,
closure_saved_names_of_captured_variables: Table<DefIndex, LazyValue<IndexVec<FieldIdx, Symbol>>>,
mir_generator_witnesses: Table<DefIndex, LazyValue<mir::GeneratorLayout<'static>>>,
promoted_mir: Table<DefIndex, LazyValue<IndexVec<mir::Promoted, mir::Body<'static>>>>,
Expand Down
24 changes: 24 additions & 0 deletions compiler/rustc_metadata/src/rmeta/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,30 @@ impl FixedSizeEncoding for bool {
}
}

impl FixedSizeEncoding for Option<bool> {
type ByteArray = [u8; 1];

#[inline]
fn from_bytes(b: &[u8; 1]) -> Self {
match b[0] {
0 => Some(false),
1 => Some(true),
2 => None,
_ => unreachable!(),
}
}

#[inline]
fn write_to_bytes(self, b: &mut [u8; 1]) {
debug_assert!(!self.is_default());
b[0] = match self {
Some(false) => 0,
Some(true) => 1,
None => 2,
};
}
}

impl FixedSizeEncoding for UnusedGenericParams {
type ByteArray = [u8; 4];

Expand Down
8 changes: 0 additions & 8 deletions compiler/rustc_middle/src/middle/codegen_fn_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,6 @@ impl CodegenFnAttrs {
}
}

/// Returns `true` if `#[inline]` or `#[inline(always)]` is present.
pub fn requests_inline(&self) -> bool {
match self.inline {
InlineAttr::Hint | InlineAttr::Always => true,
InlineAttr::None | InlineAttr::Never => false,
}
}

/// Returns `true` if it looks like this symbol needs to be exported, for example:
///
/// * `#[no_mangle]` is present
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2202,6 +2202,11 @@ rustc_queries! {
query generics_require_sized_self(def_id: DefId) -> bool {
desc { "check whether the item has a `where Self: Sized` bound" }
}

query cross_crate_inlinable(def_id: DefId) -> bool {
desc { "whether the item should be made inlinable across crates" }
separate_provide_extern
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super fond of the name.
Suggestion: export_optimized_mir?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This query is used to determine 4 things:

  • If we codegen the function in all CGUs that reference it
  • If we should encode MIR for the function
  • If the function is eligible to be inlined in MIR
  • If reachabilty analysis should recurse through the function

We do not have name for this collective group of effects or properties. The names cross_crate_inlinable and export_optimized_mir just call attention to one aspect or another. People looking for better final codegen will care about this query even if MIR inlining is disabled, and people looking for better compile times will care about this query even if LLVM inlining is disabled.

Also, this adds an unstable flag -Zcross-crate-inline-threshold which I fully expect people to use at some point (when the cost model here is a bit better) in order to fix missing LLVM inlining. I do not think that calling the flag -Zexport-optimized-mir-threshold or having different names for the flag and the query is an improvement.

I would really like to have a name for the whole concept here.

}

rustc_query_append! { define_callbacks! }
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,16 +245,15 @@ impl<'tcx> InstanceDef<'tcx> {
// drops of `Option::None` before LTO. We also respect the intent of
// `#[inline]` on `Drop::drop` implementations.
return ty.ty_adt_def().map_or(true, |adt_def| {
adt_def.destructor(tcx).map_or_else(
|| adt_def.is_enum(),
|dtor| tcx.codegen_fn_attrs(dtor.did).requests_inline(),
)
adt_def
.destructor(tcx)
.map_or_else(|| adt_def.is_enum(), |dtor| tcx.cross_crate_inlinable(dtor.did))
});
}
if let ty::InstanceDef::ThreadLocalShim(..) = *self {
return false;
}
tcx.codegen_fn_attrs(self.def_id()).requests_inline()
tcx.cross_crate_inlinable(self.def_id())
}

pub fn requires_caller_location(&self, tcx: TyCtxt<'_>) -> bool {
Expand Down
119 changes: 119 additions & 0 deletions compiler/rustc_mir_transform/src/cross_crate_inline.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
use rustc_attr::InlineAttr;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::LocalDefId;
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::*;
use rustc_middle::query::Providers;
use rustc_middle::ty::TyCtxt;
use rustc_session::config::OptLevel;

pub fn provide(providers: &mut Providers) {
providers.cross_crate_inlinable = cross_crate_inlinable;
}

fn cross_crate_inlinable(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
let codegen_fn_attrs = tcx.codegen_fn_attrs(def_id);
// If this has an extern indicator, then this function is globally shared and thus will not
// generate cgu-internal copies which would make it cross-crate inlinable.
if codegen_fn_attrs.contains_extern_indicator() {
return false;
}

// Obey source annotations first; this is important because it means we can use
// #[inline(never)] to force code generation.
match codegen_fn_attrs.inline {
InlineAttr::Never => return false,
InlineAttr::Hint | InlineAttr::Always => return true,
_ => {}
}

// This just reproduces the logic from Instance::requires_inline.
match tcx.def_kind(def_id) {
DefKind::Ctor(..) | DefKind::Closure => return true,
DefKind::Fn | DefKind::AssocFn => {}
_ => return false,
}

// Don't do any inference when incremental compilation is enabled; the additional inlining that
// inference permits also creates more work for small edits.
if tcx.sess.opts.incremental.is_some() {
return false;
}

// Don't do any inference unless optimizations are enabled.
if matches!(tcx.sess.opts.optimize, OptLevel::No) {
return false;
}

if !tcx.is_mir_available(def_id) {
return false;
}

let mir = tcx.optimized_mir(def_id);
let mut checker =
CostChecker { tcx, callee_body: mir, calls: 0, statements: 0, landing_pads: 0, resumes: 0 };
checker.visit_body(mir);
checker.calls == 0
&& checker.resumes == 0
&& checker.landing_pads == 0
&& checker.statements
<= tcx.sess.opts.unstable_opts.cross_crate_inline_threshold.unwrap_or(100)
}

struct CostChecker<'b, 'tcx> {
cjgillot marked this conversation as resolved.
Show resolved Hide resolved
tcx: TyCtxt<'tcx>,
callee_body: &'b Body<'tcx>,
calls: usize,
statements: usize,
landing_pads: usize,
resumes: usize,
}

impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> {
fn visit_statement(&mut self, statement: &Statement<'tcx>, _: Location) {
// Don't count StorageLive/StorageDead in the inlining cost.
match statement.kind {
StatementKind::StorageLive(_)
| StatementKind::StorageDead(_)
| StatementKind::Deinit(_)
| StatementKind::Nop => {}
_ => self.statements += 1,
}
}

fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, _: Location) {
let tcx = self.tcx;
match terminator.kind {
TerminatorKind::Drop { ref place, unwind, .. } => {
let ty = place.ty(self.callee_body, tcx).ty;
if !ty.is_trivially_pure_clone_copy() {
self.calls += 1;
if let UnwindAction::Cleanup(_) = unwind {
self.landing_pads += 1;
}
}
}
TerminatorKind::Call { unwind, .. } => {
self.calls += 1;
if let UnwindAction::Cleanup(_) = unwind {
self.landing_pads += 1;
}
}
TerminatorKind::Assert { unwind, .. } => {
self.calls += 1;
if let UnwindAction::Cleanup(_) = unwind {
self.landing_pads += 1;
}
}
TerminatorKind::UnwindResume => self.resumes += 1,
TerminatorKind::InlineAsm { unwind, .. } => {
self.statements += 1;
if let UnwindAction::Cleanup(_) = unwind {
self.landing_pads += 1;
}
}
TerminatorKind::Return => {}
_ => self.statements += 1,
}
}
}
14 changes: 9 additions & 5 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,11 @@ impl<'tcx> Inliner<'tcx> {
caller_body: &mut Body<'tcx>,
callsite: &CallSite<'tcx>,
) -> Result<std::ops::Range<BasicBlock>, &'static str> {
self.check_mir_is_available(caller_body, &callsite.callee)?;

let callee_attrs = self.tcx.codegen_fn_attrs(callsite.callee.def_id());
self.check_codegen_attributes(callsite, callee_attrs)?;
let cross_crate_inlinable = self.tcx.cross_crate_inlinable(callsite.callee.def_id());
self.check_codegen_attributes(callsite, callee_attrs, cross_crate_inlinable)?;

let terminator = caller_body[callsite.block].terminator.as_ref().unwrap();
let TerminatorKind::Call { args, destination, .. } = &terminator.kind else { bug!() };
Expand All @@ -183,9 +186,8 @@ impl<'tcx> Inliner<'tcx> {
}
}

self.check_mir_is_available(caller_body, &callsite.callee)?;
let callee_body = try_instance_mir(self.tcx, callsite.callee.def)?;
self.check_mir_body(callsite, callee_body, callee_attrs)?;
self.check_mir_body(callsite, callee_body, callee_attrs, cross_crate_inlinable)?;

if !self.tcx.consider_optimizing(|| {
format!("Inline {:?} into {:?}", callsite.callee, caller_body.source)
Expand Down Expand Up @@ -401,6 +403,7 @@ impl<'tcx> Inliner<'tcx> {
&self,
callsite: &CallSite<'tcx>,
callee_attrs: &CodegenFnAttrs,
cross_crate_inlinable: bool,
) -> Result<(), &'static str> {
if let InlineAttr::Never = callee_attrs.inline {
return Err("never inline hint");
Expand All @@ -414,7 +417,7 @@ impl<'tcx> Inliner<'tcx> {
.non_erasable_generics(self.tcx, callsite.callee.def_id())
.next()
.is_some();
if !is_generic && !callee_attrs.requests_inline() {
if !is_generic && !cross_crate_inlinable {
return Err("not exported");
}

Expand Down Expand Up @@ -456,10 +459,11 @@ impl<'tcx> Inliner<'tcx> {
callsite: &CallSite<'tcx>,
callee_body: &Body<'tcx>,
callee_attrs: &CodegenFnAttrs,
cross_crate_inlinable: bool,
) -> Result<(), &'static str> {
let tcx = self.tcx;

let mut threshold = if callee_attrs.requests_inline() {
let mut threshold = if cross_crate_inlinable {
self.tcx.sess.opts.unstable_opts.inline_mir_hint_threshold.unwrap_or(100)
} else {
self.tcx.sess.opts.unstable_opts.inline_mir_threshold.unwrap_or(50)
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ mod const_prop;
mod const_prop_lint;
mod copy_prop;
mod coverage;
mod cross_crate_inline;
mod ctfe_limit;
mod dataflow_const_prop;
mod dead_store_elimination;
Expand Down Expand Up @@ -123,6 +124,7 @@ pub fn provide(providers: &mut Providers) {
coverage::query::provide(providers);
ffi_unwind_calls::provide(providers);
shim::provide(providers);
cross_crate_inline::provide(providers);
*providers = Providers {
mir_keys,
mir_const,
Expand Down
Loading
Loading