Make derefer work everwhere

[ouz-a]

Follow up work on previous PR's #95649 and #95857.

r? rust-lang/mir-opt

Co-Authored-By: @oli-obk

[oli-obk]

r? @oli-obk

Reviewing even though I co-authored as my contribution was a mir opt test

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 5364c86 with merge 030e8caecdf11e51a4e0bbc897649f0deadcaef7...

[bors]

☀️ Try build successful - checks-actions
Build commit: 030e8caecdf11e51a4e0bbc897649f0deadcaef7 (030e8caecdf11e51a4e0bbc897649f0deadcaef7)

[rust-timer]

Queued 030e8caecdf11e51a4e0bbc897649f0deadcaef7 with parent 8305398, future comparison URL.

[rust-timer]

Finished benchmarking commit (030e8caecdf11e51a4e0bbc897649f0deadcaef7): comparison url.

Summary:

• Primary benchmarks: 🎉 relevant improvements found
• Secondary benchmarks: mixed results

	Regressions 😿 (primary)	Regressions 😿 (secondary)	Improvements 🎉 (primary)	Improvements 🎉 (secondary)	All 😿 🎉 (primary)
count1	0	6	2	6	2
mean2	N/A	2.0%	-1.1%	-0.4%	-1.1%
max	N/A	2.3%	-1.7%	-0.5%	-1.7%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Footnotes

1. number of relevant changes ↩

2. the arithmetic mean of the percent change ↩

[oli-obk]

Const eval now has more statements to chew through, so this perf change is expected.

@bors r+

[oli-obk]

It shouldn't have, if it does, we'll deploy the appropriate fixes immediately instead of when we move this to before borrowck

[RalfJung]

All right. I added this transmute as a testcase to Miri as well so we don't miss if/when this happens.

[RalfJung]

However, I should note that conceptually, giving the temporary for *p the type &mut T is not quite right -- we don't actually own it at that type there. The type &T would be more accurate.

Basically, we have to ensure that all passes that run after this just don't care about the type of a reference stored in a local -- & and &mut must make no difference, since this pass makes types broken. That's why moving this before Retag would be an issue, but I imagine this could also be a problem for other passes so I think this should be approached more systematically.

[oli-obk]

we have to ensure that all passes that run after this just don't care about the type of a reference stored in a local -- & and &mut must make no difference, since this pass makes types broken.

It does at present. Before actually moving it earlier we were considering various strategies. While we could start messing with types and using different ones, that seems suboptimal. What we came up with in the compiler brainstorm meetings during rust all hands was more of a "mark these locals in a way that analysis passes can understand". The types are correct as far as I can tell, it's just that the locals are more temporary than regular temporaries.

The extreme alternative would be to replace all such locals' types with raw pointers which will make these analyses ignore them mostly. We'll still need markers for borrowck though, even in that case

[RalfJung]

&mut is not a correct type, at least not under the notions of "correct" I would usually apply to MIR.

For example, if we were to obtain such MIR as input to a formal methods tool like Prusti or Creusot, we'd definitely not want to try to treat this as a mutable reference. That would fail spectacularly.

[oli-obk]

@bors retry

Makes sense, we'll move this pass before retag and emit the correct types before doing any further actions then.

[bors]

⌛ Testing commit 5364c86 with merge 055bf4c...

[bors]

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 055bf4c to master...

[rust-timer]

Finished benchmarking commit (055bf4c): comparison url.

Summary:

• Primary benchmarks: mixed results
• Secondary benchmarks: 😿 relevant regressions found

	Regressions 😿 (primary)	Regressions 😿 (secondary)	Improvements 🎉 (primary)	Improvements 🎉 (secondary)	All 😿 🎉 (primary)
count1	8	16	1	0	9
mean2	0.5%	1.2%	-1.7%	N/A	0.2%
max	0.5%	2.5%	-1.7%	N/A	-1.7%

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression

Footnotes

1. number of relevant changes ↩

2. the arithmetic mean of the percent change ↩

[oli-obk]

All of the primary regressions and the tt-muncher secondary regressions are similar to

 20,293,864  ???:<rustc_expand::mbe::macro_parser::TtParser>::parse_tt
 17,155,452  ???:<alloc::rc::Rc<alloc::vec::Vec<(rustc_ast::tokenstream::TokenTree, rustc_ast::tokenstream::Spacing)>> as core::ops::drop::Drop>::drop
-12,949,133  ???:<alloc::vec::Vec<(rustc_ast::tokenstream::TokenTree, rustc_ast::tokenstream::Spacing)> as core::ops::drop::Drop>::drop

This looks to me like just some LLVM inliner noise. We do have more local variables now, which LLVM previously just had as intermediate LLVM Values that were rarely (if ever) put into a local. This seems to me like a problem that we could address in ways beyond fixing this PR's regression. After all, if the user writes out the temporary local variables manually instead of us expanding their code, then they would see the same issues.

The other major regression in a secondary benchmark (the const eval stress test) is expected, the const evaluator has to process more statements, and every statement has overhead. Similar to the LLVM case, there are avenues to explore here to make const eval faster. I believe there are still some low hanging fruit that will yield measurable improvements, but we need to dig into benchmark dumps to find out where exactly they lie.

@rustbot label: +perf-regression-triaged