T Compiler Meeting Agenda 2022 08 25

T-compiler Meeting Agenda 2022-08-25

Announcements

Other WG meetings

MCPs/FCPs

  • New MCPs (take a look, see if you like them!)
  • Old MCPs (not seconded, take a look)
    • “Arbitrary annotations in compiletest” compiler-team#513 (last review activity: 3 months ago)
      • any pending concerns?
    • “Add support for the LoongArch architecture” compiler-team#518 (last review activity: 2 months ago)
      • MCP proponents are working on implementing this in rust#96971 (with some comments from @Wesley Wiser), rustup PR rustup#3052 and others. Was the MCP implicitely seconded?
    • “Add #[alias] attribute to allow symbol aliasing” compiler-team#526 (last review activity: about 34 days ago)
  • Pending FCP requests (check your boxes!)
  • Things in FCP (make sure you’re good with it)
  • Accepted MCPs
  • Finalized FCPs (disposition merge)
    • “Stabilize -Zgcc-ld=lld as -Clink-self-contained=linker -Clinker-flavor=gcc-lldcompiler-team#510
    • “Tracking issue for RFC 2046, label-break-value” rust#48594
    • “Tracking Issue for “unsafe blocks in unsafe fn” (RFC #2585)” rust#71668
    • “Increase the minimum linux-gnu versions” rust#95026
    • “Remove migrate borrowck mode” rust#95565
    • “Modify MIR building to drop repeat expressions with length zero” rust#95953
    • “Lang: Stabilize usage of rustc_nonnull_optimization_guaranteed on -1” rust#97122
    • “Remove a back-compat hack on lazy TAIT” rust#97346
    • “Make outlives::{components,verify} agree” rust#97406
    • “[RFC] Support .comment section like GCC/Clang (!llvm.ident)” rust#97550
    • “make cenum_impl_drop_cast deny-by-default” rust#97652
    • “make const_err show up in future breakage reports” rust#97743
    • “lub: don’t bail out due to empty binders” rust#97867
    • “allow unions with mutable references and tuples of allowed types” rust#97995
    • “do not mark interior mutable shared refs as dereferenceable” rust#98017
    • “session: stabilize split debuginfo on linux” rust#98051
    • “relate closure_substs.parent_substs() to parent fn in NLL” rust#98835

WG checkins

  • Types team by @nikomatsakis and @Jack Huey (previous checkin):

    It’s been a slow last couple weeks, since people have been on vacation. Progress continues on the various initiatives. No big updates worth mentioning.

  • @_WG-mir-opt MIR Optimizations by @oli (previous checkin):

    Checkin text

Backport nominations

T-compiler beta / T-compiler stable

  • :beta: “Check projection types before inlining MIR” rust#100571
    • PR authored and nominated by @cjgillot
    • fixes rust#100550, P-critical that broke building a crate
  • 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 hidden since WIP or waiting on other teams)

Oldest PRs waiting for review

T-compiler

  • “[DO NOT MERGE] Only apply ProceduralMasquerade hack to older versions of rentalrust#94063 (last review activity: 6 months ago)
    • cc @Aaron Hill
  • “Suggest using an appropriate keyword for struct and enumrust#94996 (last review activity: 5 months ago)
    • cc: @Esteban Küber who reviewed
  • “Rewrite LLVM’s archive writer in Rust” rust#97485 (last review activity: about 57 days ago)
    • cc @Wesley Wiser
  • “Use only ty::Unevaluated<‘tcx, ()> in type system” rust#98588 (last review activity: about 50 days ago)
    • some merge conflicts with upstream but otherwise seems reviewable again? cc: @lcnr
  • “Lower the assume intrinsic to a MIR statement” rust#98332 (last review activity: about 43 days ago)
    • cc @Wesley Wiser
  • “distinguish the method and associated function diagnostic information” rust#99064 (last review activity: about 39 days ago)
    • cc: @Esteban Küber

Issues of Note

Short Summary

P-critical

T-compiler

  • “Wrong cast of u16 to usize on aarch64” rust#97463
    • Will be fixed by rust#97800 (@Wesley Wiser approved, now waiting on bors)
  • “Regression in consteval: error[E0080]: could not evaluate static initializer (unable to turn pointer into raw bytes)” rust#99923

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
    • @_TaKO8Ki self-assigned

Unassigned P-high nightly regressions

Performance logs

triage logs for 2022-08-24

Overall some really impressive wins this week. Note in particular PR #100209, “Lazily decode SourceFile from metadata” (which improved 75 primary benchmark scenarios and 158 secondary scenarios) and PR #98655 “Don’t derive PartialEq::ne”, which improved 65 primary scenarios and 27 secondary scenarios). There were a few cases that pnkfelix explicitly decided not to mark as triaged; see report for more details there. Also pnkfelix wonders if there is a recent slight-upward trend on max-rss for the past week, see the summary graph

Triage done by @pnkfelix. Revision range: 14a459bf..4a24f08b

Summary:

(instructions:u)meanrangecount
Regressions (primary)0.6%[0.4%, 0.8%]27
Regressions (secondary)0.4%[0.2%, 0.6%]9
Improvements (primary)-1.7%[-20.1%, -0.3%]91
Improvements (secondary)-3.6%[-18.7%, -0.3%]160
All (primary)-1.2%[-20.1%, 0.8%]118

3 Regressions, 4 Improvements, 4 Mixed; 3 of them in rollups 43 artifact comparisons made in total 30 Untriaged Pull Requests

Regressions

Rollup of 15 pull requests #100677 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.3%[0.3%, 0.3%]2
Regressions (secondary)1.3%[0.5%, 1.9%]4
Improvements (primary)--0
Improvements (secondary)--0
All (primary)0.3%[0.3%, 0.3%]2
  • lqd hypothesized this was caused by PR #100652 “Remove deferred sized checks (make them eager)”
  • regressions for #100652 include most of the rollup regressions, all by similar amounts (only ucd was absent from the narrower view).
  • left a comment on PR #100652 and marked it as a regression; marked rollup as triaged.

rustc_metadata: dedupe strings to prevent multiple copies in rmeta/query cache blow file size #98851 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.6%[0.3%, 1.5%]15
Regressions (secondary)1.1%[0.3%, 1.6%]21
Improvements (primary)--0
Improvements (secondary)-0.4%[-0.6%, -0.2%]2
All (primary)0.6%[0.3%, 1.5%]15
  • the performance of this PR was heavily evaluated as part of its development.
  • some regression to instruction counts is compensated for by the improvements file-size and to max-rss.
  • the follow-up PR #100803 is going to more than compensate for the regressions here.
  • marked as triaged.

implied bounds: explicitly state which types are assumed to be wf #100676 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.3%[0.2%, 0.5%]22
Regressions (secondary)0.4%[0.2%, 0.8%]22
Improvements (primary)--0
Improvements (secondary)--0
All (primary)0.3%[0.2%, 0.5%]22
  • This PR was intended to be a refactor, but it turns out it has other problems (see issue 100910).
  • The regressions alone are not cause to revert the PR, but the soundness bug pushes me over the line.
  • Nominated for discussion (of revert) in Thursday’s T-compiler meeting. Not tagging as triaged.

Improvements

Don’t derive PartialEq::ne. #98655 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)1.0%[1.0%, 1.0%]1
Improvements (primary)-0.7%[-1.4%, -0.2%]65
Improvements (secondary)-5.2%[-10.0%, -0.3%]27
All (primary)-0.7%[-1.4%, -0.2%]65

Lazily decode SourceFile from metadata #100209 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.2%[0.2%, 0.2%]2
Regressions (secondary)0.7%[0.4%, 0.9%]3
Improvements (primary)-1.6%[-19.6%, -0.2%]75
Improvements (secondary)-3.0%[-18.3%, -0.2%]158
All (primary)-1.6%[-19.6%, 0.2%]77
  • Don’t get too excited y’all, that 19.6% improvement was to helloworld.
  • having said that, this does represent a huge win across a broad suite of benchmarks, nearly all in incremental.
  • (also, lqd notes that helloworld is a useful proxy for near-trivial build.rs scripts.)

Update minifier version to 0.2.2 #100624 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)--0
Improvements (primary)-0.6%[-1.6%, -0.3%]13
Improvements (secondary)-1.1%[-1.5%, -0.3%]20
All (primary)-0.6%[-1.6%, -0.3%]13
  • As noted by nnethercote, the cycles and max-rss results are neutral or under noise threshold, while instruction counts improved.

Kind-less SessionDiagnostic derive #100765 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)--0
Improvements (primary)-0.8%[-0.9%, -0.6%]5
Improvements (secondary)--0
All (primary)-0.8%[-0.9%, -0.6%]5
  • all five improvements are to instances of regex-opt-incr-{patched,full} benchmark

Mixed

Rollup of 9 pull requests #100810 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)0.3%[0.2%, 0.3%]2
Improvements (primary)-0.7%[-0.9%, -0.3%]8
Improvements (secondary)--0
All (primary)-0.7%[-0.9%, -0.3%]8
  • already triaged: “The small number of small improvements slightly outweighs the small number of small regressions. No further action is needed.”

update Miri #100841 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.8%[0.5%, 1.0%]5
Regressions (secondary)0.2%[0.2%, 0.2%]1
Improvements (primary)-0.8%[-0.8%, -0.8%]1
Improvements (secondary)--0
All (primary)0.5%[-0.8%, 1.0%]6
  • regressions were in regex-opt-incr-patched (and ucd-doc-full, but that was just 0.24%)
  • while the regression here is unfortunate, there is not much we can expect to do in the short term to address it
  • its not even clear whether miri is really at fault; the detailed query info says that the regression is due to LLVM_lto_optimize. Could the miri changes have somehow caused the codegen unit partitioning to change? Why would a miri update affect the time for LLVM_lto_optimize?
  • not marking as triaged. I’m not sure if anyone can justify spending time to look at this, but I don’t want to just let it slide through just yet.

Rollup of 11 pull requests #100847 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)0.2%[0.2%, 0.2%]1
Improvements (primary)-0.5%[-0.5%, -0.5%]1
Improvements (secondary)-0.7%[-1.1%, -0.3%]5
All (primary)-0.5%[-0.5%, -0.5%]1
  • benefits here heavily outweigh the one minor regression.
  • already triaged by nnethercote

Use AttrVec more #100668 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.6%[0.2%, 1.0%]2
Regressions (secondary)0.6%[0.3%, 1.3%]8
Improvements (primary)-0.3%[-0.4%, -0.2%]7
Improvements (secondary)-0.5%[-1.2%, -0.2%]15
All (primary)-0.1%[-0.4%, 1.0%]9
  • already triaged by nnethercote: “a few small wins and losses here, which balance each other out, and the net effect is perf-neutral.”

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-polymorphization by @davidtwco
  • @_WG-rls2.0 by @matklad