Enable mutable noalias for LLVM >= 12

[nikic]

Enable mutable noalias by default on LLVM 12, as previously known miscompiles have been resolved. Now it's time to find the next one ;)

• The -Z mutable-noalias option no longer has an explicit default and accepts -Z mutable-noalias=yes and -Z mutable-noalias=no to override the LLVM version based default behavior.
• The decision on whether to apply the noalias attribute is moved into rustc_codegen_llvm. rustc_middle only provides us with the necessary information to make the decision.
• noalias is not emitted for types that are !Unpin, as a heuristic for self-referential structures (see Enable noalias annotations #54878 and Resolve unsound interaction between noalias and self-referential data (incl. generators, async fn) #63818).

[nikic]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit f8452a5159f665c77df589e44b37fa17fa508df0 with merge 3352d8ec2c800d702ecbbdc68f5502978773c4f3...

[bors]

☀️ Try build successful - checks-actions
Build commit: 3352d8ec2c800d702ecbbdc68f5502978773c4f3 (3352d8ec2c800d702ecbbdc68f5502978773c4f3)

[rust-timer]

Queued 3352d8ec2c800d702ecbbdc68f5502978773c4f3 with parent 51748a8, future comparison URL.

[rust-timer]

Finished benchmarking try commit (3352d8ec2c800d702ecbbdc68f5502978773c4f3): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

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

[bjorn3]

When looking at incr-unchanged to exclude any slowdown in LLVM, there are many improvements of up to 1.4%, a few slowdowns of up to 0.6% and a regression of ctfe-stress of 1.8%.

When looking at all benchmarks, there are big losses of up to 3.3%. For the style-servo-opt full run for example LLVM_module_codegen took 28.5% longer.

[joshtriplett]

I don't think we should expect this to be compile-time-neutral; LLVM does have to do more work. It'd be helpful to have runtime performance benchmarks to compare these to. Ultimately, we may end up trading compile-time for runtime here.

[bjorn3]

It'd be helpful to have runtime performance benchmarks to compare these to.

I was looking at incr-unchanged for that. Those bail out before LLVM gets invoked.

[joshtriplett]

@bjorn3 I meant "runtime performance" as in "performance of the compiled code". I'd expect the primary benefit of noalias to be better code generation, not faster code generation.

[panstromek]

@joshtriplett I think that was the idea. If you enable noalias for the compiler, you expect the Rust part of it to get faster, so incr-unchanged can roughly tell you that.

[nikic]

I think this is ready for review now, r? @nagisa

I followed @RalfJung's suggestion to not apply noalias to !Unpin, which should avoid issues with self-referential structures in practice, though the more general T-lang problem remains open.

[nagisa]

Note that when looking at the performance results you can open the -Zself-profile outputs to see where the regressions end up being. For example: old vs new.

There are two areas of comptime regressions here: one is in LLVM internals (which makes sense as LLVM is doing strictly more work optimizing things now) and the other in generating the IR (which also makes sense as we're generating more IR)

This LGTM to me overall. r=me with or without @RalfJung's comment addressed.

[nikic]

It's probably worthwhile to check the perf results again with the full implementation. The previous perf run was with it unconditionally enabled, without the Unpin and version checks.

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit ee2eefeecbc5a8af4c5e81a05bffdad00912f3f8 with merge 989a6de73b924cf690879f45fab0a3889c267b43...

[bors]

☀️ Try build successful - checks-actions
Build commit: 989a6de73b924cf690879f45fab0a3889c267b43 (989a6de73b924cf690879f45fab0a3889c267b43)

[rust-timer]

Queued 989a6de73b924cf690879f45fab0a3889c267b43 with parent eb95ace, future comparison URL.

[bjorn3]

Yes, noalias is enabled by default after this PR. The perf runs are on a rustc version that is built using a rustc version built using the same sources as this PR: The bootstrap rustc builds a rustc which itself builds a rustc. This last rustc is what is shipped to the user and what is used for perf runs.

[felix91gr]

Ah, perfect. Thank you, @bjorn3 💛

[the8472]

I don't think we should expect this to be compile-time-neutral; LLVM does have to do more work. It'd be helpful to have runtime performance benchmarks to compare these to. Ultimately, we may end up trading compile-time for runtime here.

Would tracking the produced binary size in perf.rlo make sense as a cheaper proxy measure instead doing full benchmarks? On the assumption that LLVM optimizing more results in things being optimized away and thus smaller code.

[memoryruins]

tracking the produced binary size in perf.rlo

relevant issue rust-lang/rustc-perf#145

[felix91gr]

I don't think we should expect this to be compile-time-neutral; LLVM does have to do more work. It'd be helpful to have runtime performance benchmarks to compare these to. Ultimately, we may end up trading compile-time for runtime here.

Would tracking the produced binary size in perf.rlo make sense as a cheaper proxy measure instead doing full benchmarks? On the assumption that LLVM optimizing more results in things being optimized away and thus smaller code.

I'm not 100% sure that that would always be the case. LLVM optimizes by default for runtime performance, and many times that would entail a larger binary size. Inlining and loop unrolling are very common examples of this. Monomorphization as well.

That said, maybe there's something related to the noalias optimizations that makes LLVM reduce the binary size. I think some testing (or asking around, although with how new the feature is, maybe not many know what it does to binary size in practice) is in order :3

[the8472]

I'm not 100% sure that that would always be the case.

correlation < 1 is expected, which is why wrote "proxy measure". I just hope it's still big enough to be useful.

[archshift]

Is there any plan to enable emitting LLVM alias.scope / noalias metadata for references not originating in function arguments?

[bjorn3]

I suspect that depends on deciding a final memory model for rust.

[archshift]

I was under the impression that there are clear cases where we can guarantee no mutable aliasing around references?

[felix91gr]

Is it plausible that you two aren't talking about the same thing?

I'm not entirely sure of what context @archshift is talking about, but I think there are indeed some contexts in which we can guarantee that.

Gui, do you think you could show us an example of what you mean?

I get the feeling that the context you're talking about is more specific than what Bjorn is talking about when they mean that the memory model is needed to know the answer.

[RalfJung]

An old closed PR is definitely not the right place for such a discussion though. :) Please take this to Zulip, IRLO, or a new issue.