T Compiler Meeting Agenda 2022 04 28

T-compiler Meeting Agenda 2022-04-28

Announcements

MCPs/FCPs

  • New MCPs (take a look, see if you like them!)
    • No new proposals this time.
  • Old MCPs (not seconded, take a look)
    • “Accept pc in place of unknown and unknown in place of pc for x86_64 and i?86 targets” compiler-team#441 (last review activity: 10 months ago)
    • “Make -Z binary-dep-depinfo the default behavior” compiler-team#464 (last review activity: 6 months ago)
    • “Tier 3 target proposal: riscv64gc-linux-android (Android target for riscv64gc)” compiler-team#472 (last review activity: 4 months ago)
    • -Dwarnings to cover all warnings” compiler-team#473 (last review activity: 4 months ago)
    • “Build-time execution sandboxing” compiler-team#475 (last review activity: 4 months ago)
    • “Dealing with type/const ambiguities” compiler-team#480 (last review activity: 3 months ago)
    • “Removing codegen logic for nvptx-nvidia-cuda (32-bit target)” compiler-team#496 (last review activity: about 41 days ago)
    • “Add attribute to run specific tests in an isolated process” compiler-team#508 (last review activity: about 1 days ago)
    • “Stabilize -Zgcc-ld=lldcompiler-team#510 (last review activity: about 1 days ago)
  • Pending FCP requests (check your boxes!)
    • “Tracking issue for Consistent no-prelude attribute (RFC 501)” rust#20561
    • “Tracking Issue for -Z terminal-widthrust#84673
    • “Increase the minimum linux-gnu versions” rust#95026
  • Things in FCP (make sure you’re good with it)
    • “Remove mutable_borrow_reservation_conflict lint and allow the code pattern” rust#96268
  • Accepted MCPs
    • No new accepted proposals this time.
  • Finalized FCPs (disposition merge)
    • “Stabilize let_chains in Rust 1.62.0” rust#94927

WG checkins

  • We no longer support the legacy pass manager with LLVM 15 (though I don’t think we have any open issues for the NewPM anymore)
  • Clang has enabled opaque pointers by default, so we’ll do that as well when we update to LLVM 15.
  • Upstream patches to support Rust’s allocator are slowly moving forward, but haven’t fully landed yet.
  • LLVM 15 raises the toolchain baseline, though we currently can still work around this using LLVM_TEMPORARILY_ALLOW_OLD_TOOLCHAIN=ON. We need https://github.com/rust-lang/rust/pull/95026 to support building new LLVM versions.
  • @_WG-traits (generic work of the WG) by @nikomatsakis and @Jack Huey (previous checkin)

Types team RFC is posted: https://github.com/rust-lang/rfcs/pull/3254 We have had deep-dive meetings on Fridays

  • So far, we’ve covered the subtyping and variance nomicon chapter PR, “Chalkification”, and new subtyping in a-mir-formality
  • Tomorrow, we have a deep dive into the new TAIT algorithm Also went through adding Copy to a-mir-formality: https://www.youtube.com/watch?v=7ZQ-9yZztKA&t=589s

Backport nominations

T-compiler beta / T-compiler stable

T-rustdoc beta / T-rustdoc stable

  • No backport nominations for T-rustdoc this time.

:back: / :shrug: / :hand:

PRs S-waiting-on-team

T-compiler

  • No PRs waiting on T-compiler this time.

Oldest PRs waiting for review

T-compiler

  • No unreviewed PRs on T-compiler at this time.

T-rustdoc stable / T-rustdoc beta

  • No beta nominations for T-rustdoc this time.
  • No stable nominations for T-rustdoc this time.

:back: / :shrug: / :hand:

PRs S-waiting-on-team

T-compiler

  • No new PRs waiting on T-compiler

Oldest PRs waiting for review

T-compiler

  • “AddNicheCases MirPass” rust#95652 (last review activity: about 23 days ago)
    • Bot assigned @Wesley Wiser as reviewer, reroll?
  • “Support tool lints with the #[expect] attribute (RFC 2383)” rust#95542 (last review activity: about 22 days ago)
    • PR author assigned review to @Wesley Wiser, reroll?
  • “Add sub_ptr on pointers (the usize version of offset_from)” rust#95837 (last review activity: about 17 days ago)
    • previous review from @RalfJ (maybe unassign from @kennytm? They were bot-assigned)
  • “Remove #[rustc_deprecated]rust#95960 (last review activity: about 16 days ago)
    • assigned review to @Josh Triplett
    • needs additional reviewer for the compiler part?

Issues of Note

Short Summary

P-critical

T-compiler

  • No P-critical issues for T-compiler this time.

T-rustdoc

  • No P-critical issues for T-rustdoc this time.

P-high regressions

P-high beta regressions

  • No P-high beta regressions this time.

Unassigned P-high nightly regressions

  • No unassigned P-high nightly regressions this time.

Performance logs

triage logs 2022-04-26

This was, in general, a positive week for compiler performance. There were many concentrated efforts on improving rustdoc performance with a lot of real world crates showing ~4-7% improvements in full build times. Additionally, there was further improvement to macro_rules! performance with many real world crates improving performance by as much as 18% in full builds! On the other hand, the regressions were mostly minor and largely relegated to secondary benchmarks.

Triage done by @rylev. Revision range: 4ca19e09d302a4cbde14f9cb1bc109179dc824cd..1c988cfa0b7f4d3bc5b1cb40dc5002f5adbfb9ad

4 Regressions, 6 Improvements, 3 Mixed; 1 of them in rollups 45 artifact comparisons made in total

Regressions

Rollup of 5 pull requests #96263 (Comparison Link)

Regressions (primary) Regressions (secondary) Improvements (primary) Improvements (secondary) All (primary)
count 2 6 0 0 2
mean 2.0% 6.3% N/A N/A 2.0%
max 3.2% 6.9% N/A N/A 3.2%
  • The regression in primary benchmarks is dominated by a noisy syn-1.0.89 (see rustc-perf#1301). However, the regressions in the secondary benchmarks seem real and point towards #96236 as the possible cause.
  • I ran cachegrind diff on wg-grammar check full and got these results which shows <rustc_borrowck::region_infer::Trace as alloc::vec::spec_from_elem::SpecFromElem>::from_elem being called a lot more often after this change.
  • The regressions are not huge but they are certainly significant and real. If something obvious stands out to those more familiar with this code, it might be worth poking around.

rustdoc: Unindent doc fragments on Attributes construction #96282 (Comparison Link)

Regressions (primary) Regressions (secondary) Improvements (primary) Improvements (secondary) All (primary)
count 1 10 0 0 1
mean 0.4% 0.4% N/A N/A 0.4%
max 0.4% 0.5% N/A N/A 0.4%
  • An issue was opened to investigate whether more work is being done that strictly necessary.
  • Since the regressions are relatively minor and only constrained to somewhat “artificial” crates (i.e., hello-world is the only primary crate impacted), we can mark this as triaged.

Generate synthetic object file to ensure all exported and used symbols participate in the linking #95604 (Comparison Link)

Regressions (primary) Regressions (secondary) Improvements (primary) Improvements (secondary) All (primary)
count 3 13 0 0 3
mean 0.4% 0.4% N/A N/A 0.4%
max 0.5% 0.5% N/A N/A 0.5%
  • This seems to be regressing a bunch of full doc builds. Given this doesn’t touch rustdoc, but does touch metadata encoding/decoding, I would assume that’s where the perf hit is coming from.
  • I ran cachegrind diff on helloworld doc full and got these results. It indeed looks like we’re calling decoding functions on certain items (attributes and idents) more than previously (albeit with less decoding of spans).
  • Left a comment to the author/reviewer to get more clarification.

Display function path in unsafety violations - E0133 #96294 (Comparison Link)

Regressions (primary) Regressions (secondary) Improvements (primary) Improvements (secondary) All (primary)
count 44 0 0 0 44
mean 1.3% N/A N/A N/A 1.3%
max 2.9% N/A N/A N/A 2.9%
  • Performance runs were done on this change before merging and they did not show perf regressions
  • Added a comment to ensure the issue doesn’t get lost. It seems it might be caused by inclusion of DefIds when they aren’t needed.

Improvements

Mixed

rustdoc: Optimize and refactor doc link resolution #96135 (Comparison Link)

Regressions (primary) Regressions (secondary) Improvements (primary) Improvements (secondary) All (primary)
count 6 0 14 20 20
mean 0.3% N/A -1.3% -0.7% -0.9%
max 0.3% N/A -2.7% -1.0% -2.7%
  • The regression in stm32f4 is expected and given this is otherwise a big perf win, we’ll take the slight perf hit in one benchmark.

Remove visibility information from HIR #93970 (Comparison Link)

Regressions (primary) Regressions (secondary) Improvements (primary) Improvements (secondary) All (primary)
count 0 16 2 2 2
mean N/A 1.3% -0.2% -0.3% -0.2%
max N/A 2.2% -0.2% -0.3% -0.2%
  • The regression happens in a stress test that is expected to stress the code being changed.
  • After this change visibility is now being correctly hashed which is strictly more work but is the “correct” thing to do. Given this the regressions are worth it.

Make derefer work everwhere #96116 (Comparison Link)

Regressions (primary) Regressions (secondary) Improvements (primary) Improvements (secondary) All (primary)
count 8 16 1 0 9
mean 0.5% 1.2% -1.7% N/A 0.2%
max 0.5% 2.5% -1.7% N/A -1.7%
  • Unfortunate regressions but they are fairly isolated, small, and expected.
  • Majority of the regressions come from the introduction of more local variables which LLVM has to work through. This looks like an area we’ll want to work through, but we shouldn’t block this PR on addressing this.

Nominated Issues

T-compiler

  • No new I-compiler-nominated issues

RFC

  • No nominated RFCs for T-compiler this time.