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

add optimization fuel checks to some mir passes #79117

Merged
merged 7 commits into from
Nov 19, 2020
Merged

Conversation

cjkenn
Copy link
Contributor

@cjkenn cjkenn commented Nov 16, 2020

Fixes #77402

Inserts a bunch of calls to consider_optimizing. Note that consider_optimizing is the method that actually decrements the fuel count, so the point at which it's called is when the optimization takes place, from a fuel perspective. This means that where we call it has some thought behind it:

  1. We probably don't want to decrement the fuel count before other simple checks, otherwise we count an optimization as being performed even if nothing was mutated (ie. it returned early).
  2. In cases like InstCombine, where we gather optimizations in a pass and then mutate values, we probably would rather skip the gathering pass for performance reasons rather than skip the mutations afterwards.

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 16, 2020
@cjkenn
Copy link
Contributor Author

cjkenn commented Nov 17, 2020

Hmm I updated that failing test to account for increased usage of fuel, but on the second CI run the used value changed. Right now I can't reproduce locally either (I can only get a fuel value of 7), so I don't thinkI know what's being run in CI...

@oli-obk
Copy link
Contributor

oli-obk commented Nov 17, 2020

r? @oli-obk

@rust-highfive rust-highfive assigned oli-obk and unassigned eddyb Nov 17, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Nov 17, 2020

That difference could be because the test is run with optimizations on your system while it's run without optimizations on CI. Try adding -C opt-level=0 to the flag list in

// compile-flags: --error-format human -Z print-fuel=foo

@oli-obk
Copy link
Contributor

oli-obk commented Nov 17, 2020

cc @rust-lang/wg-mir-opt

@oli-obk oli-obk added the A-mir-opt Area: MIR optimizations label Nov 17, 2020
@oli-obk
Copy link
Contributor

oli-obk commented Nov 19, 2020

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Nov 19, 2020

📌 Commit 07de702 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 19, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 19, 2020
Rollup of 14 pull requests

Successful merges:

 - rust-lang#78961 (Make bad "rust-call" arguments no longer ICE)
 - rust-lang#79082 (Improve the diagnostic for when an `fn` contains qualifiers inside an `extern` block.)
 - rust-lang#79090 (libary: Forward compiler-builtins "asm"  and "mangled-names" feature)
 - rust-lang#79094 (Add //ignore-macos to pretty-std-collections.rs)
 - rust-lang#79101 (Don't special case constant operands when lowering intrinsics)
 - rust-lang#79102 (Add two regression tests)
 - rust-lang#79110 (Remove redundant notes in E0275)
 - rust-lang#79116 (compiletest: Fix a warning in debuginfo tests on windows-gnu)
 - rust-lang#79117 (add optimization fuel checks to some mir passes)
 - rust-lang#79147 (Highlight MIR as Rust on GitHub)
 - rust-lang#79149 (Move capture lowering from THIR to MIR)
 - rust-lang#79155 (fix handling the default config for profiler and sanitizers)
 - rust-lang#79156 (Allow using `download-ci-llvm` from directories other than the root)
 - rust-lang#79164 (Permit standalone generic parameters as const generic arguments in macros)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2fdcd24 into rust-lang:master Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mir-opt Area: MIR optimizations S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make all mir optimizations respect the optimization fuel
7 participants