Add flags customizing behaviour of MIR inlining #78873
Conversation
rust-highfive
added
the
S-waiting-on-review
| @@ -17,9 +17,6 @@ use std::collections::VecDeque; | |||
| use std::iter; | |||
| use std::ops::RangeFrom; | |||
|
|
|||
| const DEFAULT_THRESHOLD: usize = 50; | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
What do you think about something like the above? Or do you have other ideas for generalizing configurable mir optimizations?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Modified the PR to add threshold flag only for now.
* `-Zinline-mir-threshold` to change the default threshold. * `-Zinline-mir-hint-threshold` to change the threshold used by functions with inline hint.
|
☔ The latest upstream changes (presumably #78904) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels: |
tmiasko
force-pushed
the
inline-opts
branch
2 times, most recently
from
150d961 to
c8943c6
Compare
|
@bors r+ |
|
📌 Commit c8943c6 has been approved by |
bors
added
S-waiting-on-bors
…as-schievink Rollup of 11 pull requests Successful merges: - rust-lang#78216 (Duration::zero() -> Duration::ZERO) - rust-lang#78354 (Support enable/disable sanitizers/profiler per target) - rust-lang#78417 (BTreeMap: split off most code of append) - rust-lang#78832 (look at assoc ct, check the type of nodes) - rust-lang#78873 (Add flags customizing behaviour of MIR inlining) - rust-lang#78899 (Support inlining diverging function calls) - rust-lang#78923 (Cleanup and comment intra-doc link pass) - rust-lang#78929 (rustc_target: Move target env "gnu" from `linux_base` to `linux_gnu_base`) - rust-lang#78930 (rustc_taret: Remove `TargetOptions::is_like_android`) - rust-lang#78942 (Fix typo in comment) - rust-lang#78947 (Ship llvm-cov through llvm-tools) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
-Zinline-mir-thresholdto change the default threshold.-Zinline-mir-hint-thresholdto change the threshold used byfunctions with inline hint.
Having those as configurable flags makes it possible to experiment with with
different inlining thresholds and substantially increase test coverage of MIR
inlining when used with increased thresholds (for example, necessary to test
#78844).