T Compiler Meeting Agenda 2022 02 24

T-compiler Meeting Agenda 2022-02-24

Tracking Issue

Announcements

  • :tada: Today, February 24th, release Rust stable 1.59 :tada: (blog post)
  • Reminder: if you see a PR/issue that seems like there might be legal implications due to copyright/IP/etc, please let the Core team know (or at least message @pnkfelix or @Wesley Wiser so we can pass it along).
  • pnkfelix note: the bootstrap process is probably the biggest tester of incremental compilation on beta channel (because beta otherwise is not commonly used for incremental development). If you are seeing any problem there atop the beta compiler, please do file a bug; it represents something that’s on the path to hitting stable.

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: 7 months ago)
    • “Make -Z binary-dep-depinfo the default behavior” compiler-team#464 (last review activity: 4 months ago)
    • “Tier 3 target proposal: riscv64gc-linux-android (Android target for riscv64gc)” compiler-team#472 (last review activity: 2 months ago)
    • “-Dwarnings to cover all warnings” compiler-team#473 (last review activity: 2 months ago)
    • “Build-time execution sandboxing” compiler-team#475 (last review activity: about 59 days ago)
    • “Dealing with type/const ambiguities” compiler-team#480 (last review activity: about 34 days ago)
  • Pending FCP requests (check your boxes!)
    • “Stabilize native library modifier syntax and the whole-archive modifier specifically” rust#93901
  • Things in FCP (make sure you’re good with it)
  • Accepted MCPs
  • Finalized FCPs (disposition merge)
    • “Check if enum from foreign crate has any non exhaustive variants when attempting a cast” rust#92744
    • “Stabilize #[cfg(panic = "...")]” rust#93658

WG checkins

No major updates since last time.

No major updates since last time, lcnr is working on a fix for #75325, the primary remaining blocker for polymorphization, hopefully resolved after current issues related to higher-ranked lifetimes are resolved.

Backport nominations

T-compiler beta / T-compiler stable

  • :beta: “No branch protection metadata unless enabled” rust#93516
    • PR authored by @nagisa
    • PR is open, reviewer is @cjgillot, didnt yet r+’d
    • nominated by @nagisa since missed the beta cut-off
  • :beta: “update auto trait lint for PhantomData” rust#94315
    • PR authored by @lcnr
    • PR is open, r+’d by @oli
    • nominated by @lcnr for backport, seems a small safe change

T-rustdoc beta / T-rustdoc stable

  • No backport nominations for T-rustdoc this time.

:back: / :shrug: / :hand:

PRs S-waiting-on-team

T-compiler

  • “Stabilize native library modifier syntax and the whole-archive modifier specifically” rust#93901
    • perhaps waiting on more :heavy_check_mark: from T-compiler (see comment)

Oldest PRs waiting for review

T-compiler

  • “When encountering a binding that could be a const or unit variant, suggest the right path” rust#90988 (last review activity: 3 months ago)
  • “Fix exposing fields marked unstable or doc hidden” rust#90358 (last review activity: 2 months ago)
    • PR is now waiting for review
    • rustbot assigned to @Esteban Küber (but first round of review was done by others)
    • maybe reassign?
  • “Improve suggestion when casting usize to (possibly) wide pointer” rust#92150 (last review activity: 2 months ago)
    • reviewer was assigned by the rustbot
    • PR author @Michael Goulet (compiler-errors) asks if assignee is confirmed or can be reassigned
  • “remove obligation dedup from impl_or_trait_obligations” rust#84944 (last review activity: 2 months ago)
    • cc: @Aaron Hill
  • “Initial implementation of transmutability trait.” rust#92268 (last review activity: about 60 days ago)
    • First round of review by @_oli
    • PR seems a bit in flux, perhaps the actual status is not ready for review?

Issues of Note

Short Summary

P-critical

T-compiler

  • “Nested mutex guards with dropping and re-assigning confuse the borrow checker” rust#93770
    • not a regression but probably worth mentioning in this meeting
  • “optimized i686 fails primal-sieve tests” #94032
    • @_pnkfelix suggests that it can be downgraded to P-high

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-02-24

A relatively quiet week with improvements largely outweighing regressions. On the regressions side, doc performance has worsened somewhat significantly most likely introduced by adding more docs to blanket impls. On the improvement side, LLVM 14 allowed for an optimization the significantly improves codegen performance.

Triage done by @rylev. Revision range: a240ccd81c74c105b6f5fe84c46f8d36edb7e306..1204400ab8da9830f6f77a5e40e7ad3ea459676a

3 Regressions, 2 Improvements, 7 Mixed; 3 of them in rollups 27 Untriaged Pull Requests 38 comparisons made in total

Regressions

Rollup of 8 pull requests #94072

  • Average relevant regression: 11.3%
  • Largest regression in instruction counts: 157.0% on full builds of stm32f4 doc
  • The regressions are all in doc runs but the largest are quite severe.
  • The most likely culprit is #89869 which adds documentation to many different blanket impls.
  • Left a comment to investigate.

Rollup of 9 pull requests #94103

  • Average relevant regression: 1.1%
  • Largest regression in instruction counts: 2.1% on full builds of deeply-nested-async check
  • Almost all of the regressions are in stress tests so the actual impact on users is likely not that large
  • None of the rolled up PRs seem to be suspicious so it’s hard to know where to begin

Fix a layout possible miscalculation in alloc::RawVec #83706

  • Average relevant regression: 0.5%
  • Largest regression in instruction counts: 1.2% on incr-unchanged builds of deeply-nested-async debug
  • A small regression would seem likely after this change since more work (e.g., checked multiplication) is being done in RawVec which is used quite a lot. I am, however, unsure whether the actual regression we’re seeing here is expected
  • Left a comment for investigation.

Improvements

  • Rollup of 7 pull requests #94254
  • Reapply cg_llvm: fewer_names in uncached_llvm_type #94107

Mixed

Upgrade to LLVM 14 #93577

  • Average relevant regression: 0.7%
  • Average relevant improvement: -1.3%
  • Largest improvement in instruction counts: -3.8% on full builds of projection-caching check
  • Largest regression in instruction counts: 3.0% on incr-patched: add static arr item builds of coercions debug
  • Upgrading LLVM is always likely to produce performance changes. Luckily the perf improvements seem to outweigh the perf regressions considerably both in number and magnitude.

Revert #91403 #94088

  • Average relevant regression: 1.1%
  • Average relevant improvement: -1.5%
  • Largest improvement in instruction counts: -1.7% on full builds of issue-88862 check
  • Largest regression in instruction counts: 1.7% on incr-unchanged builds of clap-rs check
  • The perf regressions here are relatively minor, and this change fixes a correctness issue, so I think it’s fine to let it through.

Guard against unwinding in cleanup code #92911

  • Average relevant regression: 0.9%
  • Average relevant improvement: -1.6%
  • Largest improvement in instruction counts: -3.4% on incr-full builds of syn opt
  • Largest regression in instruction counts: 3.7% on full builds of ripgrep opt
  • A relatively large regression considering the change is meant to protect against a rare occurrence (double unwind).
  • Left a comment asking for justification.

Allow inlining of ensure_sufficient_stack() #93934

  • Average relevant regression: 0.9%
  • Average relevant improvement: -0.8%
  • Largest improvement in instruction counts: -1.0% on incr-patched: add vec item builds of deep-vector opt
  • Largest regression in instruction counts: 0.9% on incr-unchanged builds of ctfe-stress-4 check
  • This was an attempt at an optimization that proved fruitful before LLVM 14 was merged. Now the regressions and improvements weigh each other out.

safely transmute<&List<Ty<'tcx>>, &List<GenericArg<'tcx>>> #93505

  • Average relevant regression: 1.2%
  • Average relevant improvement: -0.6%
  • Largest improvement in instruction counts: -0.8% on incr-full builds of ucd check
  • Largest regression in instruction counts: 2.7% on full builds of deeply-nested-async check
  • This led to a larger regression than was seen before the PR was merged.
  • The author is now looking into the perf

Simplify rustc_serialize by dropping support for decoding into JSON #93839

  • Average relevant regression: 0.8%
  • Average relevant improvement: -0.5%
  • Largest improvement in instruction counts: -0.5% on incr-unchanged builds of ctfe-stress-4 check
  • Largest regression in instruction counts: 1.0% on full builds of externs opt
  • This change was justified: The performance changes to the compiler are pretty much a wash, but this does have a good impact on compiler bootstrapping (~6 seconds).

Introduce ChunkedBitSet and use it for some dataflow analyses. #93984

  • Average relevant regression: 1.0%
  • Average relevant improvement: -3.8%
  • Largest improvement in instruction counts: -5.3% on full builds of keccak debug
  • Largest regression in instruction counts: 6.0% on full builds of clap-rs check
  • While the regressions look minor they are likely even less of an issue due to this particular optimization likely helping wall-time and definitely helping max RSS while hurting instruction counts.
  • For more detail see the justification.

Nominated Issues

T-compiler

  • “Stabilize guaranteed compile time evaluation of unnamed constant items” rust#93838
  • “Stabilize native library modifier syntax and the whole-archive modifier specifically” rust#93901
  • “process: release procedure only runs crater on nightly->beta cut” rust#94266

RFC

  • No nominated RFCs for T-compiler this time.