T Compiler Meeting Agenda 2022 09 22

T-compiler Meeting Agenda 2022-09-22

Announcements

Other WG meetings

MCPs/FCPs

  • New MCPs (take a look, see if you like them!)
    • No new proposals this time.
  • Old MCPs (not seconded, take a look)
    • “Arbitrary annotations in compiletest” compiler-team#513 (last review activity: 4 months ago)
    • “Add #[alias] attribute to allow symbol aliasing” compiler-team#526 (last review activity: 2 months ago)
    • “Use RangeInclusive in SpanData instead of lo/hi” compiler-team#534 (last review activity: about 27 days ago)
    • “Allow informational -Z flags on stable compiler” compiler-team#542 (last review activity: about 27 days ago)
    • " Promote i586-unknown-linux-gnu to Tier 2 with Host Tools " compiler-team#543 (last review activity: about 27 days ago)
    • “Lower baseline expectations for i686 unix-like targets” compiler-team#548 (last review activity: about 13 days ago)
    • “MCP: Flag to disable extended error info.” compiler-team#550 (last review activity: about 13 days ago)
    • “New Tier-3 target proposal: powerpc64-ibm-aix” compiler-team#553 (last review activity: about 13 days ago)
    • “configurable rustc timeout for compiletest tests” compiler-team#554 (last review activity: about 13 days ago)
    • “MCP: Raise UEFI Targets to Tier-2” compiler-team#555 (last review activity: about 13 days ago)
    • “Raise minimum supported macOS and iOS versions” compiler-team#556 (last review activity: about 13 days ago)
    • “Rustc Contributor Program Major Change Proposal” compiler-team#557 (last review activity: about 0 days ago)
  • Pending FCP requests (check your boxes!)
  • Things in FCP (make sure you’re good with it)
  • Accepted MCPs
    • No new accepted proposals this time.
  • Finalized FCPs (disposition merge)
    • “Tracking issue for ..X, and ..=X (#![feature(half_open_range_patterns)])” rust#67264
    • “Tracking Issue for #[instruction_set] attribute (RFC 2867)” rust#74727
    • “Stabilize let else” rust#93628
    • “Stabilize generic associated types” rust#96709
    • “Make typeck aware of uninhabited types” rust#100288
    • “Consider #[must_use] annotation on async fn as also affecting the Future::Output” rust#100633
    • “Commit to safety rules for dyn trait upcasting” rust#101336

WG checkins

  • @_WG-rustc-dev-guide by @Santiago Pastorino and @Yuki Okushi|217081 (previous checkin):

    Most notable changes

    • Document multipart_suggestion derive on SessionSubdiagnostic #1446
    • Document changes introduced by kind-less SessionDiagnostics #1440
    • diagnostics: fix outdated use of string slugs #1436

    Most notable WIPs

    • Add a review checklist and suggest reviews as a way to get started with the project #1463
    • date-check: updating-llvm #1424
    • Added detail to codegen section #1216
    • Document inert vs active attributes #1110

  • Impl Trait initiative by @oli (previous checkin):

    we had a deep dive on TAIT stabilization status: https://hackmd.io/FNLjLrevQcSKoIDAsf9ivQ TLDR: 5 issues left to fix, then we could stabilize. missing things are either “X doesn’t compile, but should” or “diagnostics are weird”

Backport nominations

T-compiler beta / T-compiler stable

  • :beta: “Avoid duplicating StorageLive in let-else” rust#101894
  • :beta: “Re-add HRTB implied static bug note” rust#101924
    • suggested by @Jack Huey (to have this along the GATs stabilization)
  • :beta: “fix ConstProp handling of written_only_inside_own_block_locals” rust#102045
    • Fixes P-critical unsoundness in rust#101973
    • (backport suggested by @RalfJ)
  • No stable nominations for T-compiler this time.

T-rustdoc beta / T-rustdoc stable

  • No backport nominations for T-rustdoc this time.

:back: / :shrug: / :hand:

PRs S-waiting-on-team

T-compiler

  • 2 issues waiting on other teams or recently discussed

Oldest PRs waiting for review

T-compiler

  • “Enable MIR inlining for generators too” rust#99782 (last review activity: about 57 days ago)
    • on the radar of @cjgillot, is it waiting on a comment for latest perf. run?
  • “Initial support for loongarch64_unknown_linux_gnuf64” rust#96971 (last review activity: about 38 days ago)
    • rust :robot: assigned @Wesley Wiser
  • “Inline SyntaxContext in both encoded span representation.” rust#98840 (last review activity: about 37 days ago)
    • can be merged? reviewer @__nnethercote approved
  • “Allow impl Fn() -> impl Trait in return position” rust#93582 (last review activity: about 36 days ago)
    • unsure about actual status. Does the author needs input?

Issues of Note

Short Summary

P-critical

T-compiler

  • “Potential miscompilation on i686 of chacha20” rust#101346
    • last week T-compiler discussion
    • Related #101861 (T-libs) has been merged
    • Still needs a fix on LLVM upstream
    • Jack commented that the submodules changes from #101861 were not yet in nightly? Are they now?
  • “E0477 triggered with current nightly” rust#101951
    • addressed in rust#102016 by @lcnr (PR is approved and is waiting on bors)

T-rustdoc

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

P-high regressions

P-high beta regressions

  • “Anon lifetime in impl Trait no longer suggests adding a lifetime parameter” rust#100615
    • discussed and added comments, this will slip into 1.64
  • “Borrow checking for static methods became more strict” rust#100725
    • this will also slip into 1.64

Unassigned P-high nightly regressions

  • “codegen regression for bool” rust#101048
    • this will be a beta regression now

Performance logs

triage logs for 2022-09-20

This was a fairly negative week for compiler performance, with regressions overall up to 14% on some workloads (primarily incr-unchanged scenarios), largely caused by #101620. We are still chasing down either a revert or a fix for that regression, though a partial mitigation in #101862 has been applied. Hopefully the full fix or revert will be part of the next triage report.

We also saw a number of other regressions land, though most were much smaller in magnitude.

Triage done by @simulacrum. Revision range: 17cbdfd07178349d0a3cecb8e7dde8f915666ced..8fd6d03e22fba2930ad377b87299de6a37076074

Summary:

(instructions:u) mean range count
Regressions (primary) 3.8% [0.3%, 14.0%] 148
Regressions (secondary) 4.3% [0.3%, 23.6%] 141
Improvements (primary) -6.4% [-24.8%, -0.5%] 24
Improvements (secondary) -2.1% [-4.0%, -0.4%] 12
All (primary) 2.4% [-24.8%, 14.0%] 172

1 Regressions, 2 Improvements, 6 Mixed; 2 of them in rollups 43 artifact comparisons made in total

Regressions

Cleanup slice sort related closures in core and alloc #101816 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 1.1% [1.1%, 1.1%] 1
Regressions (secondary) - - 0
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 1.1% [1.1%, 1.1%] 1

This regression occurred in a single benchmark and only in the incremental scenario, so it isn’t worth follow up investigation at this time.

Improvements

Partially revert #101433 #101902 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.5% [-0.7%, -0.4%] 6
Improvements (secondary) -0.6% [-0.6%, -0.6%] 1
All (primary) -0.5% [-0.7%, -0.4%] 6

Do not fetch HIR node when iterating to find lint. #101862 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.6% [-2.3%, -0.2%] 24
Improvements (secondary) -13.1% [-39.1%, -3.2%] 6
All (primary) -0.6% [-2.3%, -0.2%] 24

Mixed

Simplify caching and storage for queries #101307 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 1.7% [1.7%, 1.7%] 1
Improvements (primary) -0.2% [-0.3%, -0.2%] 7
Improvements (secondary) -0.3% [-0.3%, -0.3%] 3
All (primary) -0.2% [-0.3%, -0.2%] 7

This change is either neutral or a slight improvement; the one regression occurs in a benchmark and only in one scenario, which is also prone to noise, so further investigation isn’t necessary.

Rollup of 6 pull requests #101805 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.4% [0.4%, 0.5%] 2
Regressions (secondary) - - 0
Improvements (primary) - - 0
Improvements (secondary) -4.5% [-5.0%, -4.1%] 6
All (primary) 0.4% [0.4%, 0.5%] 2

Using the newish rollup-introspection tooling, @nnethercote narrowed this down to #101433. Put a comment on that PR and marked it as a perf-regression.

Compute lint levels by definition #101620 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 4.1% [0.4%, 14.5%] 133
Regressions (secondary) 5.2% [0.2%, 64.1%] 131
Improvements (primary) -5.1% [-24.3%, -0.4%] 30
Improvements (secondary) -1.0% [-1.2%, -0.5%] 5
All (primary) 2.4% [-24.3%, 14.5%] 163

Regression is partially addressed by #101862, but not completely. Week/week diff remains negative. Left a comment asking us to pursue the revert for the time being, since the suggested fix for this regression didn’t recover performance fully.

Derive various impls instead of hand-rolling them #101858 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 1.2% [0.2%, 2.2%] 11
Regressions (secondary) 0.9% [0.3%, 2.1%] 18
Improvements (primary) - - 0
Improvements (secondary) -0.7% [-0.9%, -0.5%] 6
All (primary) 1.2% [0.2%, 2.2%] 11

This was a fairly large regression in a number of benchmarks; #101893 is pursuing further investigation here and tracking either improvements or a possible revert. It seems like moving to the derive’d impls may have also been a (correct) behavior change in some cases so a straightforward revert may be undesirable.

Change FnMutDelegate to trait objects #101857 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 1.2% [0.9%, 1.6%] 9
Improvements (primary) -1.2% [-1.2%, -1.2%] 1
Improvements (secondary) - - 0
All (primary) -1.2% [-1.2%, -1.2%] 1

Regressions are limited to secondary benchmarks and stress tests at that. The goal here is an improvement in bootstrap times (measured at ~1.5% earlier, though the actual PR merge was during a window where that measurement was failing). That improvement is worth the slight loss on stress tests.

Rollup of 7 pull requests #101949 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.2% [0.2%, 0.2%] 3
Regressions (secondary) 0.8% [0.2%, 3.5%] 15
Improvements (primary) - - 0
Improvements (secondary) -1.4% [-1.6%, -1.3%] 6
All (primary) 0.2% [0.2%, 0.2%] 3

Still working on identifying the causes. Likely to be a minor enough delta that spending too much more time won’t make sense.

Nominated Issues

T-compiler

  • No nominated issues for T-compiler this time.

RFC

  • No nominated RFCs for T-compiler this time.

Next week’s WG checkins

  • @_WG-llvm by @nagisa and @Nikita Popov
  • Types team by @nikomatsakis and @Jack Huey