T Compiler Meeting Agenda 2022 06 09

T-compiler Meeting Agenda 2022-06-09

Announcements

Other WG meetings (calendar link)

MCPs/FCPs

  • New MCPs (take a look, see if you like them!)
    • No new proposals this time.
  • Old MCPs (not seconded, take a look)
    • “Tier 3 target proposal: riscv64gc-linux-android (Android target for riscv64gc)” compiler-team#472 (last review activity: 5 months ago)
      • :loudspeaker: Stale MCP: will be closed next week
      • Zulip discussion
      • MCP author replied to all questions, so it seems no visible concern to be addressed
    • -Dwarnings to cover all warnings” compiler-team#473 (last review activity: 5 months ago)
      • :loudspeaker: Stale MCP: will be closed next week
      • Zulip discussion
      • Comments seem to underline possible conflicts with cargo linting system.
    • “Dealing with type/const ambiguities” compiler-team#480 (last review activity: 4 months ago)
    • “Removing codegen logic for nvptx-nvidia-cuda (32-bit target)” compiler-team#496 (last review activity: 2 months ago)
    • “Arbitrary annotations in compiletest” compiler-team#513 (last review activity: about 24 days ago)
  • Pending FCP requests (check your boxes!)
    • “Stabilize -Zgcc-ld=lld as -Clink-self-contained=linker -Clinker-flavor=gcc-lldcompiler-team#510
    • “Increase the minimum linux-gnu versions” rust#95026
      • Note: T-libs discussed and approved this change (comment)
  • Things in FCP (make sure you’re good with it)
    • No FCP requests this time.
  • Accepted MCPs
  • Finalized FCPs (disposition merge)
    • “Tracking Issue for -Z terminal-widthrust#84673
    • “Remove migrate borrowck mode” rust#95565
    • “Stabilize the bundle native library modifier” rust#95818
    • “Modify MIR building to drop repeat expressions with length zero” rust#95953
    • “Remove label/lifetime shadowing warnings” rust#96296
    • “Lang: Stabilize usage of rustc_nonnull_optimization_guaranteed on -1” rust#97122

WG checkins

  • @_WG-traits Traits (generic work of the WG) by @nikomatsakis and @Jack Huey (previous checkin)

Officially renamed to types team. Steady progress on various initiatives. Biggest update is migrate mode removal. Over this month, we’re going to do triage during deep dives.

Checkin text

Backport nominations

T-compiler beta / T-compiler stable

  • :beta: “Update LLVM submodule” rust#97690
    • Fixes rust#97428, P-critical unsoundness bug
    • author @Nikita Popov mentions that LLVM upgrade should be safe (comment)
  • :beta: “Fail gracefully when encountering an HRTB in APIT. " rust#97683
    • Fixes rust#96954, a P-medium ICE that should be an error (like in stable)
    • @nagisa nominated (comment)
  • :beta: “Aarch64 call abi does not zeroext (and one cannot assume it does so)” rust#97800
    • Fixes a P-critical unsoundness bug on tier 1 platform
    • patch author and backport nomination by @pnkfelix
  • :stable: “Aarch64 call abi does not zeroext (and one cannot assume it does so)” rust#97800
    • also stable backport, comment clarifies the nomination: if we had to do a point release for other reasons, should this be included?

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.
    • (4 PRs omitted because either drafts or waiting on other teams / RFC process)

Oldest PRs waiting for review

T-compiler

  • “reduce RPC overhead for common proc_macro operations” rust#86822
    • nominated by @nnethercote (comment)
    • PR is also I-compiler-nominated
  • “Add option to pass environment variables” rust#94387 (last review activity: about 33 days ago)
    • cc latest reviewers: @Esteban Küber and @bjorn3
  • Clone suggestions” rust#95115 (last review activity: about 26 days ago)
    • assigned reviewer @Esteban Küber, suggests an additional reviewer (comment)
    • also cc: @oli and @Jak{e,ob} Degen (as they left comments, too)
    • any actionable for the PR author at this time?
  • “Add Finalize statement to make deaggregation “reversible” by storing all information in MIR” rust#96043 (last review activity: about 25 days ago)
    • unsure about status: maybe a comment on latest perf run results?
    • cc: @Wesley Wiser since assigned reviewer
  • “Print type of every call in a method call chain” rust#96918 (last review activity: about 20 days ago)
    • unsure about current status
    • cc: @Michael Goulet (compiler-errors) maybe to comment on latest notes left by @Esteban Küber?
  • “Only compile #[used] as llvm.compiler.used for ELF targets” rust#93718 (last review activity: about 55 days ago)
    • seems ready for another round of review
    • cc @pnkfelix

Issues of Note

Short Summary

P-critical

T-compiler

  • “Infinite recursion optimized away with opt-level=zrust#97428
    • Fixed by upgrading to LLVM/14.x (#97690)
    • #97690 incorporates fix by @Nikita Popov comment
  • “Wrong cast of u16 to usize on aarch64” rust#97463
    • Fixed by #97800

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 for 2022-06-07

A busy week in compiler performance, but fortunately improvements outweighed regressions. The biggest improvements came from @nnethercote’s work on making the decoding of SourceFile::lines lazy which significantly cuts the costs of decoding crate metadata. The biggest regressions came from the removal of json handling in rustc_serialize which has been a multi-month effort to improve the maintainability of json (de-)serialization in the compiler.

Triage done by @rylev. Revision range: 0a43923a..bb55bd

Summary:

mean max count
Regressions (primary) 0.5% 3.2% 36
Regressions (secondary) 0.3% 0.9% 15
Improvements (primary) -1.3% -15.1% 124
Improvements (secondary) -2.7% -13.5% 182
All (primary) -0.9% -15.1% 160

2 Regression, 6 Improvements, 5 Mixed; 4 of them in rollups 48 artifact comparisons made in total

Regressions

rewrite error handling for unresolved inference vars #89862 (Comparison Link)

mean max count
Regressions (primary) 0.2% 0.3% 7
Regressions (secondary) 0.4% 1.0% 23
Improvements (primary) N/A N/A 0
Improvements (secondary) N/A N/A 0
All (primary) 0.2% 0.3% 7
  • Ran cache grind diff and saw that ObligationForest::process_obligations is getting called a lot more.
  • I’m very unfamiliar with trait resolution, so I’m unsure if this is a red herring or not.
  • In any case, here is the full diff.
  • Left a comment as such here

Remove all json handling from rustc_serialize #85993 (Comparison Link)

mean max count
Regressions (primary) 0.5% 1.3% 68
Regressions (secondary) 0.9% 3.5% 40
Improvements (primary) -0.4% -0.6% 3
Improvements (secondary) -0.5% -1.0% 24
All (primary) 0.5% 1.3% 71
  • @nnethercote was not able to reproduce the regressions in a local build, and it seems the consensus is that the regressions are worth the hit.

Improvements

Make params be SmallVec as originally was #97670 (Comparison Link)

mean max count
Regressions (primary) N/A N/A 0
Regressions (secondary) N/A N/A 0
Improvements (primary) -0.2% -0.2% 3
Improvements (secondary) N/A N/A 0
All (primary) -0.2% -0.2% 3

Add PID to LLVM PGO profile path #97137 (Comparison Link)

mean max count
Regressions (primary) N/A N/A 0
Regressions (secondary) N/A N/A 0
Improvements (primary) -0.6% -0.8% 20
Improvements (secondary) -0.7% -1.0% 8
All (primary) -0.6% -0.8% 20

Rollup of 6 pull requests #97742 (Comparison Link)

mean max count
Regressions (primary) N/A N/A 0
Regressions (secondary) N/A N/A 0
Improvements (primary) N/A N/A 0
Improvements (secondary) -1.9% -2.6% 12
All (primary) N/A N/A 0

interpret: better control over whether we read data with provenance #97684 (Comparison Link)

mean max count
Regressions (primary) N/A N/A 0
Regressions (secondary) N/A N/A 0
Improvements (primary) -0.7% -1.9% 8
Improvements (secondary) -5.5% -10.5% 12
All (primary) -0.7% -1.9% 8

Remove migrate borrowck mode #95565 (Comparison Link)

mean max count
Regressions (primary) N/A N/A 0
Regressions (secondary) N/A N/A 0
Improvements (primary) -0.5% -1.9% 47
Improvements (secondary) -0.5% -1.4% 21
All (primary) -0.5% -1.9% 47

Lazify SourceFile::lines. #97575 (Comparison Link)

mean max count
Regressions (primary) N/A N/A 0
Regressions (secondary) 0.4% 0.5% 6
Improvements (primary) -1.8% -15.3% 52
Improvements (secondary) -2.9% -13.8% 124
All (primary) -1.8% -15.3% 52

Mixed

Add #[inline] to Vec’s Deref/DerefMut #97553 (Comparison Link)

mean max count
Regressions (primary) 0.5% 0.8% 6
Regressions (secondary) 0.4% 0.7% 31
Improvements (primary) -1.2% -1.7% 10
Improvements (secondary) -1.1% -1.9% 10
All (primary) -0.5% -1.7% 16
  • As with any chance to inlining, performance is expected to change and to not always have a positive impact.
  • The improvements outweigh the regressions (especially in primary benchmarks), and so it doesn’t seem worth it to dig too much more into this.

Rollup of 6 pull requests #97644 (Comparison Link)

mean max count
Regressions (primary) N/A N/A 0
Regressions (secondary) 0.6% 0.7% 5
Improvements (primary) -0.2% -0.3% 2
Improvements (secondary) -0.3% -0.5% 11
All (primary) -0.2% -0.3% 2
  • The improvements outweigh the regressions (which are pretty small and contained in secondary benchmarks).
  • Given it’s in a rollup, it’s not worth the effort to investigate.

Rollup of 5 pull requests #97654 (Comparison Link)

mean max count
Regressions (primary) N/A N/A 0
Regressions (secondary) 0.6% 0.6% 1
Improvements (primary) N/A N/A 0
Improvements (secondary) -0.6% -0.8% 8
All (primary) N/A N/A 0
  • The improvements outweigh the regressions (which are pretty small and contained in secondary benchmarks).
  • Given it’s in a rollup, it’s not worth the effort to investigate.

Rollup of 3 pull requests #97694 (Comparison Link)

mean max count
Regressions (primary) 0.2% 0.2% 1
Regressions (secondary) 0.3% 0.5% 5
Improvements (primary) -0.4% -0.6% 12
Improvements (secondary) -0.3% -0.5% 13
All (primary) -0.4% -0.6% 13
  • The improvements outweigh the regressions (which are pretty small and almost completely contained in secondary benchmarks).
  • Given it’s in a rollup, it’s not worth the effort to investigate.

Inline bridge::Buffer methods. #97604 (Comparison Link)

mean max count
Regressions (primary) 3.8% 3.8% 1
Regressions (secondary) N/A N/A 0
Improvements (primary) -0.3% -0.3% 1
Improvements (secondary) -2.1% -2.1% 3
All (primary) 1.8% 3.8% 2
  • I’m a bit confused as this PR was opened to address a performance regression, but it seems to be itself a partial perf regression (at least in instruction counts).
  • This causes a near 4% perf regression in serde_derive-1.0.136. Granted that particular benchmark has been somewhat noisy but not nearly to the level seen here.
  • Add a comment seeking justification

Nominated Issues

T-compiler

  • “reduce RPC overhead for common proc_macro operations” rust#86822
    • nominated by @nnethercote (comment)
    • points out better perf results and that the PR is ready for review
  • “sess: stabilize --terminal-widthrust#95635
    • nominated by @oli for a mention for T-compiler
  • “Update Mac Catalyst support for Clang 13” rust#96392
    • nominated by @Esteban Küber in comment to find someone more conversant with that platform
    • @__Thom Chiovoloni pinged a list of names that could help (comment)

RFC

  • “New rustc and Cargo options to allow path sanitisation by default” rfcs#3127
    • @Andy Wang asked for T-compiler to sign-off the changes (comment)
    • @bjorn3 followed with a review
    • related T-compiler meeting compiler-team#516

Next week’s WG checkins