Remove the deduplicate_blocks pass #136786
Conversation
rustbot
added
S-waiting-on-review
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
compiler-errors
force-pushed
the
de-de-duplicate-blocks
branch
from
acdcb20 to
a6dcfe3
Compare
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
|
Oh wait, this is gated behind opt level 4 anyways. So it shouldn't be perf sensitive anyways. |
…ks, r=<try> Remove the deduplicate_blocks pass I don't think this pass does anything. It's a lot of complexity for 🤷 amount of benefit. r? oli-obk
|
If this is imposing maintenance burden elsewhere on the compiler it should definitely be deleted. The pass looks like a reasonable optimization to have, but in practice MIR contains a lot of blocks that contain the same statements but acting on different locals. Storage markers especially make blocks that look similar to the eye ineligible for this pass. |
|
It's not really posing a maintenance burden, it just seems to me like it's both in practice pretty useless and would need to be reworked so much (to properly implement the right "modulo" we want for deduplicating blocks like this) that there's no reason keeping it around. But if you disagree and are happy with keeping it around, feel free to close this PR. |
|
I don't have any strong feelings. But I wonder if we should reap other passes that are also only enabled at high MIR opt levels. |
|
I was planning on doing that soon, too. |
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
This comment was marked as off-topic.
This comment was marked as off-topic.
rustbot
removed
the
S-waiting-on-perf
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#135285 (it-self → itself, build-system → build system, type-alias → type alias) - rust-lang#135677 (Small `rustc_resolve` cleanups) - rust-lang#136239 (show supported register classes in error message) - rust-lang#136246 (include note on variance and example) - rust-lang#136354 (Update docs for impl keyword) - rust-lang#136786 (Remove the deduplicate_blocks pass) - rust-lang#136833 (compiler: die immediately instead of handling unknown target codegen) - rust-lang#136847 (Simplify intra-crate qualifiers.) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#135285 (it-self → itself, build-system → build system, type-alias → type alias) - rust-lang#135677 (Small `rustc_resolve` cleanups) - rust-lang#136239 (show supported register classes in error message) - rust-lang#136246 (include note on variance and example) - rust-lang#136354 (Update docs for impl keyword) - rust-lang#136786 (Remove the deduplicate_blocks pass) - rust-lang#136833 (compiler: die immediately instead of handling unknown target codegen) - rust-lang#136847 (Simplify intra-crate qualifiers.) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#136786 - compiler-errors:de-de-duplicate-blocks, r=oli-obk Remove the deduplicate_blocks pass I don't think this pass does anything. It's a lot of complexity for 🤷 amount of benefit. r? oli-obk
I would think we'd establish that by doing a perf run with it enabled by default. I'm quite surprised none of us thought to do that, but at least I'm pretty confident that (considering the pass basically never finds anything to deduplicate) it would be a no-op. |
I don't think this pass does anything. It's a lot of complexity for 🤷 amount of benefit.
r? oli-obk