T-compiler Meeting Agenda 2024-02-29

Announcements

  • Reminder: if you see a PR/issue that seems like there might be legal implications due to copyright/IP/etc, please let us know (or at least message @davidtwco or @Wesley Wiser so we can pass it along).

MCPs/FCPs

WG checkins

Backport nominations

Note: we are ~20 days from the release cut, if you have anything to backport feel free to label as such!

T-compiler beta / T-compiler stable

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

T-types stable / T-types beta

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

PRs S-waiting-on-team

T-compiler

Issues of Note

Short Summary

P-critical

T-compiler

  • "regression: visibility qualifiers are not permitted" rust#121607
    • Regression in a beta crater run, affects a number of correlated crates, not particularly widespread nor updated but the minimal repro looks pretty simple (comment)
    • Fine to also downgrade the P- label
    • @León Orell Liehr (fmease) already posted a patch #120698, assigned to @Michael (compiler-errors) Goulet for review (thanks to both!)

T-types

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

P-high regressions

P-high beta regressions

  • No P-high beta regressions this time.

Unassigned P-high nightly regressions

  • "Severe perf regression in optimized debug builds due to extra UB checks" rust#121245
    • from last week, meeting time ran out, postponed to today
    • suggestion from Wesley to land Nils' PR #121114 (comment)

Performance logs

triage logs for 2024-02-7

A rare week where regressions out powered improvements to make the compiler roughly half a percent slower on average on nearly 100 benchmarks. Some regressions have fixes in the pipeline, but some remain elusive or were introduced to address correctness issues.

Triage done by @rylev.
Revision range: 5af21304..71ffdf7f

Summary:

(instructions:u) mean range count
Regressions (primary) 1.0% [0.2%, 4.4%] 69
Regressions (secondary) 1.4% [0.2%, 4.9%] 66
Improvements (primary) -1.1% [-3.3%, -0.2%] 28
Improvements (secondary) -0.6% [-1.5%, -0.2%] 33
All (primary) 0.4% [-3.3%, 4.4%] 97

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

Regressions

Always evaluate free constants and statics, even if previous errors occurred #121087 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.8% [0.4%, 2.0%] 4
Regressions (secondary) 1.0% [0.3%, 5.2%] 11
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.8% [0.4%, 2.0%] 4
  • Regressions in primary benchmarks are real and addressed in #121387
  • Regressions in secondary benchmarks seem to be all noise.

Use intrinsics::debug_assertions in debug_assert_nounwind #120863 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.8% [0.3%, 1.7%] 13
Regressions (secondary) - - 0
Improvements (primary) -0.4% [-0.4%, -0.4%] 1
Improvements (secondary) - - 0
All (primary) 0.7% [-0.4%, 1.7%] 14
  • Pinged the author on what next steps are. Regression is in codegen which would be expected given the nature of the change, but it's unclear how one would address the issue.

wasm: Store rlib metadata in wasm object files #120588 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 3.1% [3.0%, 3.3%] 4
Regressions (secondary) 1.5% [0.2%, 2.9%] 33
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 3.1% [3.0%, 3.3%] 4
  • Regressions seem legit but nothing in self profiling stands out as a likely culprit.
  • Asked the author for more investigation.

Subtree update of rust-analyzer #121581 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.7% [0.7%, 0.8%] 4
Regressions (secondary) 0.4% [0.2%, 0.7%] 23
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.7% [0.7%, 0.8%] 4
  • This is all noise - regressions revert in the next run.

Improvements

Improve codegen diagnostic handling #121129 (Comparison Link)

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

remove sub_relations from the InferCtxt #119989 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.3% [-0.5%, -0.2%] 28
Improvements (secondary) -0.4% [-0.7%, -0.2%] 28
All (primary) -0.3% [-0.5%, -0.2%] 28

Rollup of 7 pull requests #121549 (Comparison Link)

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

Use br instead of a conditional when switching on a constant boolean #120650 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.9% [-2.2%, -0.1%] 19
Improvements (secondary) - - 0
All (primary) -0.9% [-2.2%, -0.1%] 19

speed up x install by skipping archiving and compression #118724 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.7% [-0.8%, -0.6%] 4
Improvements (secondary) -0.4% [-0.7%, -0.2%] 24
All (primary) -0.7% [-0.8%, -0.6%] 4

Use generic NonZero in tests. #121461 (Comparison Link)

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

Mixed

Rollup of 8 pull requests #121345 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.6% [0.4%, 1.1%] 9
Regressions (secondary) 1.7% [1.7%, 1.7%] 1
Improvements (primary) -0.4% [-0.8%, -0.2%] 6
Improvements (secondary) -0.8% [-2.1%, -0.3%] 17
All (primary) 0.2% [-0.8%, 1.1%] 15
  • Lots of seemingly risky PRs in this roll up. Need to run through them.

match lowering: eagerly simplify match pairs #120904 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 0.2% [0.2%, 0.2%] 3
Improvements (primary) -2.0% [-3.1%, -0.2%] 7
Improvements (secondary) - - 0
All (primary) -2.0% [-3.1%, -0.2%] 7
  • Improvements far outweigh the regressions so I don't think this needs further investigation.

compiler: clippy::complexity fixes #121523 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 1.5% [0.2%, 2.7%] 2
Regressions (secondary) - - 0
Improvements (primary) - - 0
Improvements (secondary) -1.2% [-1.2%, -1.2%] 1
All (primary) 1.5% [0.2%, 2.7%] 2
  • The large regression that caused this to be marked overall as a regression is just noise.

Add #[rustc_no_mir_inline] for standard library UB checks #121114 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.2%, 1.2%] 33
Regressions (secondary) 1.5% [0.3%, 5.5%] 12
Improvements (primary) -0.3% [-0.3%, -0.3%] 3
Improvements (secondary) -0.3% [-0.3%, -0.2%] 8
All (primary) 0.4% [-0.3%, 1.2%] 36
  • Consider necessary for fixing some pathological performance cases as well as addressing another critical issue. (See this analysis for more detail.)

Implement RFC 3373: Avoid non-local definitions in functions #120393 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 2.2% [0.5%, 4.2%] 12
Regressions (secondary) 0.7% [0.1%, 1.8%] 5
Improvements (primary) - - 0
Improvements (secondary) -0.4% [-0.8%, -0.1%] 27
All (primary) 2.2% [0.5%, 4.2%] 12
  • #121625 is meant to try to address this.
  • Given the complex nature of the change, I imagine a revert is not desirable even if performance can't be gained back.

Nominated Issues

T-compiler

  • "debuginfo: Stabilize -Z debug-macros, -Z collapse-macro-debuginfo and #[collapse_debuginfo]" rust#120845
    • posted in a past meeting (comment)
    • @Jack Huey left some comments
    • nominated by @Vadim Petrochenkov, opened an FCP. Anything more to discuss or can the nomination be removed?

RFC

  • No I-compiler-nominated RFCs this time.

Oldest PRs waiting for review

T-compiler

  • "compiler: allow transmute of ZST arrays with generics" rust#114009 (last review activity: 7 months ago)
    • cc @Wesley Wiser (or are you ok with passing this to someone else?)
  • "Create the previous dep graph index on a background thread" rust#116375 (last review activity: 4 months ago)
    • cc: cjgillot
  • "Fix ICE from zero-length span when suggesting to remove trailing semi-colon from final statement in block" rust#118953 (last review activity: 2 months ago)
    • cc @cjgillot

Next week's WG checkins

Skipping next week

Next meetings' agenda draft: hackmd link

Select a repo