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.

Sign up for GitHub

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

Add flags customizing behaviour of MIR inlining #78873

Merged
merged 1 commit into from
Nov 12, 2020
Merged

Add flags customizing behaviour of MIR inlining #78873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions compiler/rustc_interface/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,8 @@ fn test_debugging_options_tracking_hash() {
tracked!(function_sections, Some(false));
tracked!(human_readable_cgu_names, true);
tracked!(inline_in_all_cgus, Some(true));
tracked!(inline_mir_threshold, 123);
tracked!(inline_mir_hint_threshold, 123);
tracked!(insert_sideeffect, true);
tracked!(instrument_coverage, true);
tracked!(instrument_mcount, true);
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_mir/src/transform/inline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ use crate::transform::MirPass;
use std::iter;
use std::ops::{Range, RangeFrom};

const DEFAULT_THRESHOLD: usize = 50;
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there are lots more "constants" (and we may want to change them in the future), maybe we could create a more general solution: A way to pass a key/value pair list to specific optimizations. This way we don't have to (re)create a command line flag for every tiny thing, but can just parse arbitrary things out of the key/value pair list.

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about something like the above? Or do you have other ideas for generalizing configurable mir optimizations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could also consider add new category of flags dedicated for MIR optimizations. I don't know if we need a completely different mechanism, especially with those flags being the first of a kind.

As far as enabling / disabling specific pass this might be better handled in context of #77665, so I could leave this out.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if we need a completely different mechanism, especially with those flags being the first of a kind.

That's fair. So we'll revisit once we get more than like 5 flags in total or sth.

As far as enabling / disabling specific pass this might be better handled in context of #77665, so I could leave this out.

If your work is blocked on this, we can merge it now and remove it again later, but otherwise leaving it out for #77665 would be best. It may just take a bit for that to happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified the PR to add threshold flag only for now.

const HINT_THRESHOLD: usize = 100;

const INSTR_COST: usize = 5;
const CALL_PENALTY: usize = 25;
const LANDINGPAD_PENALTY: usize = 50;
Expand Down Expand Up @@ -248,7 +245,11 @@ impl Inliner<'tcx> {
}
}

let mut threshold = if hinted { HINT_THRESHOLD } else { DEFAULT_THRESHOLD };
let mut threshold = if hinted {
self.tcx.sess.opts.debugging_opts.inline_mir_hint_threshold
} else {
self.tcx.sess.opts.debugging_opts.inline_mir_threshold
};

// Significantly lower the threshold for inlining cold functions
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::COLD) {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_session/src/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,10 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
(default: no)"),
incremental_verify_ich: bool = (false, parse_bool, [UNTRACKED],
"verify incr. comp. hashes of green query instances (default: no)"),
inline_mir_threshold: usize = (50, parse_uint, [TRACKED],
"a default MIR inlining threshold (default: 50)"),
inline_mir_hint_threshold: usize = (100, parse_uint, [TRACKED],
"inlining threshold for functions with inline hint (default: 100)"),
inline_in_all_cgus: Option<bool> = (None, parse_opt_bool, [TRACKED],
"control whether `#[inline]` functions are in all CGUs"),
input_stats: bool = (false, parse_bool, [UNTRACKED],
Expand Down
19 changes: 19 additions & 0 deletions src/test/mir-opt/inline/inline-options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Checks that inlining threshold can be controlled with
// inline-mir-threshold and inline-hint-threshold options.
//
// compile-flags: -Zinline-mir-threshold=90
// compile-flags: -Zinline-mir-hint-threshold=50

// EMIT_MIR inline_options.main.Inline.after.mir
fn main() {
not_inlined();
inlined::<u32>();
}

// Cost is approximately 3 * 25 + 5 = 80.
#[inline]
pub fn not_inlined() { g(); g(); g(); }
pub fn inlined<T>() { g(); g(); g(); }

#[inline(never)]
fn g() {}
56 changes: 56 additions & 0 deletions src/test/mir-opt/inline/inline_options.main.Inline.after.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// MIR for `main` after Inline

fn main() -> () {
let mut _0: (); // return place in scope 0 at $DIR/inline-options.rs:8:11: 8:11
let _1: (); // in scope 0 at $DIR/inline-options.rs:9:5: 9:18
let _2: (); // in scope 0 at $DIR/inline-options.rs:10:5: 10:21
scope 1 (inlined inlined::<u32>) { // at $DIR/inline-options.rs:10:5: 10:21
let _3: (); // in scope 1 at $DIR/inline-options.rs:10:5: 10:21
let _4: (); // in scope 1 at $DIR/inline-options.rs:10:5: 10:21
let _5: (); // in scope 1 at $DIR/inline-options.rs:10:5: 10:21
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/inline-options.rs:9:5: 9:18
_1 = not_inlined() -> bb1; // scope 0 at $DIR/inline-options.rs:9:5: 9:18
// mir::Constant
// + span: $DIR/inline-options.rs:9:5: 9:16
// + literal: Const { ty: fn() {not_inlined}, val: Value(Scalar(<ZST>)) }
}

bb1: {
StorageDead(_1); // scope 0 at $DIR/inline-options.rs:9:18: 9:19
StorageLive(_2); // scope 0 at $DIR/inline-options.rs:10:5: 10:21
StorageLive(_3); // scope 1 at $DIR/inline-options.rs:10:5: 10:21
_3 = g() -> bb2; // scope 1 at $DIR/inline-options.rs:10:5: 10:21
// mir::Constant
// + span: $DIR/inline-options.rs:10:5: 10:21
// + literal: Const { ty: fn() {g}, val: Value(Scalar(<ZST>)) }
}

bb2: {
StorageDead(_3); // scope 1 at $DIR/inline-options.rs:10:5: 10:21
StorageLive(_4); // scope 1 at $DIR/inline-options.rs:10:5: 10:21
_4 = g() -> bb3; // scope 1 at $DIR/inline-options.rs:10:5: 10:21
// mir::Constant
// + span: $DIR/inline-options.rs:10:5: 10:21
// + literal: Const { ty: fn() {g}, val: Value(Scalar(<ZST>)) }
}

bb3: {
StorageDead(_4); // scope 1 at $DIR/inline-options.rs:10:5: 10:21
StorageLive(_5); // scope 1 at $DIR/inline-options.rs:10:5: 10:21
_5 = g() -> bb4; // scope 1 at $DIR/inline-options.rs:10:5: 10:21
// mir::Constant
// + span: $DIR/inline-options.rs:10:5: 10:21
// + literal: Const { ty: fn() {g}, val: Value(Scalar(<ZST>)) }
}

bb4: {
StorageDead(_5); // scope 1 at $DIR/inline-options.rs:10:5: 10:21
_2 = const (); // scope 1 at $DIR/inline-options.rs:10:5: 10:21
StorageDead(_2); // scope 0 at $DIR/inline-options.rs:10:21: 10:22
_0 = const (); // scope 0 at $DIR/inline-options.rs:8:11: 11:2
return; // scope 0 at $DIR/inline-options.rs:11:2: 11:2
}
}