T Compiler Meeting Agenda 2024 03 14

T-compiler Meeting Agenda 2024-03-14

Announcements

  • :loudspeaker: Next week release of stable 1.77 rustc :loudspeaker:
  • 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).

Other WG meetings

MCPs/FCPs

WG checkins

Next week’s WG checkins

  • @_WG-async-foundations by @nikomatsakis and @tmandry (previous checkin):

    Checkin text

  • @_WG-diagnostics by @Esteban Küber and @oli (previous checkin):

    #[diagnostic::on_unimplemented] shipped to stable!

Backport nominations

T-compiler beta / T-compiler stable

  • :beta: [1.77?] “Downgrade const eval dangling ptr in final to future incompat lint” rust#122204
    • Authored by Felix: “Short term band-aid for issue #121610 (P-high regression in a crater run), downgrading the prior hard error to a future-incompat lint (tracked in issue #122153).”
    • already merged, nominated by Wesley
    • perf. run looks neutral
  • 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

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

T-types

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

P-high regressions

P-high beta regressions

  • “rust-1.75.0 fails to compile with ICE on aarch64 and various ppc arches with LTO enabled - error: could not compile memchr” rust#121124
    • Waiting on feedback from Gentoo folks (issue is progressing)
  • “regression: visibility qualifiers are not permitted” rust#121607
    • fixed by #122004 (beta backport approved last week)
  • “regression: encountered mutable pointer in final value” rust#121610
    • Will be fixed by #122204 (beta nominated)

Unassigned P-high nightly regressions

  • No unassigned P-high nightly regressions this time.

Performance logs

triage logs 2024-03-11

A mixed week, with a vast number of improvements (in large part due to PR #122010, which undoes a prior regression; PR #120985, a host LLVM update). But also three admittedly small-ish regressions which seemed unanticipated and were still large enough that I did not feel comfortable rubber-stamping them with a perf-regression-triaged marking.

Triage done by @pnkfelix. Revision range: 41d97c8a..e919669d

Summary:

(instructions:u)meanrangecount
Regressions (primary)0.6%[0.2%, 1.4%]38
Regressions (secondary)1.1%[0.2%, 4.9%]50
Improvements (primary)-1.0%[-4.8%, -0.2%]119
Improvements (secondary)-0.8%[-2.2%, -0.4%]36
All (primary)-0.6%[-4.8%, 1.4%]157

2 Regressions, 5 Improvements, 9 Mixed; 5 of them in rollups 54 artifact comparisons made in total

Regressions

interpret: avoid a long-lived PlaceTy in stack frames #121985 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.3%[0.3%, 0.3%]1
Regressions (secondary)3.0%[0.2%, 4.5%]8
Improvements (primary)--0
Improvements (secondary)--0
All (primary)0.3%[0.3%, 0.3%]1
  • primary regression was to html5ever doc-full; was anctipated during development and is presumed spurious.
  • marking as triaged.

Detect unused struct impls pub trait #121752 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.4%[0.2%, 0.5%]6
Regressions (secondary)--0
Improvements (primary)--0
Improvements (secondary)--0
All (primary)0.4%[0.2%, 0.5%]6
  • primary regressions are all to serde and cranelift codegen for various profiles of incr-patched:println.
  • the cycles measurement didn’t observe any change at all, but that could be due to the difference being swamped by overall variance
  • the perf diff highlights that the query live_symbols_and_ignored_derived_traits is the source of the perf regression, which is consistent with the idea that this lint has become more expensive since that’s where we see the call to the newly-added solve_rest_impl_items (a worklist algorithm from the PR).
  • leaving a note for the author about this; not marking as triaged.

Improvements

Rollup of 7 pull requests #122111 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)--0
Improvements (primary)-0.4%[-0.4%, -0.3%]7
Improvements (secondary)-0.5%[-0.6%, -0.3%]5
All (primary)-0.4%[-0.4%, -0.3%]7
  • all 7 primary improvements are to html5ever.
  • all 5 secondary improvements are to tt-muncher.

Rollup of 8 pull requests #122117 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)--0
Improvements (primary)-2.1%[-3.9%, -0.4%]12
Improvements (secondary)--0
All (primary)-2.1%[-3.9%, -0.4%]12
  • all 12 primary improvements are to diesel
  • this is because of PR #122107, which made the non_local_definitions lint allow-by-default
  • this effectively had the reverse effect of PR #120393 (which added the aforementioned lint and caused regressions to 12 variations of diesel).

Merge collect_mod_item_types query into check_well_formed #121500 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)--0
Improvements (primary)-0.6%[-0.8%, -0.2%]4
Improvements (secondary)-0.5%[-0.6%, -0.5%]2
All (primary)-0.6%[-0.8%, -0.2%]4
  • 3 significant primary improvements are to libc incr-patched:clone, and 1 less significant to bitmaps check incr-unchanged.

Avoid invoking the intrinsic query for DefKinds other than Fn or AssocFn #122010 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)--0
Improvements (primary)-0.5%[-1.0%, -0.2%]74
Improvements (secondary)-0.7%[-2.1%, -0.2%]26
All (primary)-0.5%[-1.0%, -0.2%]74
  • undoes the vast bulk of the broad perf regression injected by PR #120675

Dep node encoding cleanups #122064 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)--0
Improvements (primary)-0.3%[-0.3%, -0.2%]19
Improvements (secondary)-0.3%[-0.4%, -0.2%]12
All (primary)-0.3%[-0.3%, -0.2%]19

Mixed

Optimize write with as_const_str for shorter code #122059 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)1.1%[0.3%, 1.9%]2
Improvements (primary)-0.8%[-1.2%, -0.4%]2
Improvements (secondary)-0.3%[-0.3%, -0.3%]1
All (primary)-0.8%[-1.2%, -0.4%]2
  • (secondary) regressions are to deep-vector debug-full (which may be spurious based on the graph) and wg-grammar debug-incr-unchanged
  • since @nnethercote was already involved in related efforts here (e.g. PR #121001) and this resulting refinement, I’m going to mark this as triaged.

Replace the default branch with an unreachable branch If it is the last variant #120268 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.7%[0.2%, 1.8%]4
Regressions (secondary)0.2%[0.2%, 0.3%]7
Improvements (primary)-0.8%[-1.2%, -0.3%]5
Improvements (secondary)-0.9%[-2.2%, -0.3%]3
All (primary)-0.1%[-1.2%, 1.8%]9
  • primary regressions are to cargo opt-full (1.76%), cargo debug-incr-patched:println (0.40%), libc doc-full (0.50%), hyper doc-full (0.19%).
  • the regression to cargo opt-full was anticipated by rust-timer runs during development; but other follow-up rust-timer runs did not always include the same regression.
  • the PR itself notes that there are known performance problems in LLVM with unreachable branches (see e.g. llvm#78578). It is not clear to me whether the above regressions are related to that.
  • posted comment asking PR author for more info. Not marking as triaged.

Rollup of 8 pull requests #122182 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)1.3%[1.3%, 1.3%]1
Regressions (secondary)0.8%[0.2%, 1.3%]2
Improvements (primary)-0.3%[-0.7%, -0.2%]17
Improvements (secondary)-0.5%[-0.9%, -0.2%]15
All (primary)-0.3%[-0.7%, 1.3%]18
  • sole primary regression is to image opt-full
  • improvements obviously outweigh the regressions
  • … but oh, there is also a bootstrap regression that may be of concern: “Bootstrap: 648.483s -> 652.903s (0.68%)”
  • kobzol has hypothesized that PR #122099 may be the cause of the bootstrap regression.
  • after some discussion on the rollup PR, decided to mark as triaged; the author may well choose to do some followup, but we will not hound them about it. :)

Replace TypeWalker usage with TypeVisitor in wf.rs #122150 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)0.7%[0.3%, 1.3%]6
Improvements (primary)-0.3%[-0.4%, -0.2%]6
Improvements (secondary)-0.2%[-0.3%, -0.2%]3
All (primary)-0.3%[-0.4%, -0.2%]6
  • six (secondary) regressions (to variants of unify-linearly and regression-31157) were anticipated during development
  • we were also expecting a broader set of 34 primary improvements. But the mean primary improvement we expected was -0.3%, which was unchanged.
  • marking as triaged

Rollup of 12 pull requests #122241 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)0.9%[0.2%, 1.6%]2
Improvements (primary)-0.3%[-0.3%, -0.3%]2
Improvements (secondary)-0.3%[-0.3%, -0.3%]1
All (primary)-0.3%[-0.3%, -0.3%]2
  • sole regressions are to (secondary) regression-31157 debug-incr-full (1.56%), which might be spurious based on the graph, and tt-muncher opt-incr-unchanged (0.19%), which I don’t consider worth getting worried about.
  • marking as triaged.

Update host LLVM on x64 Linux to LLVM 18 #120985 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.3%[0.1%, 1.0%]88
Regressions (secondary)0.4%[0.1%, 0.5%]21
Improvements (primary)-1.0%[-1.6%, -0.4%]54
Improvements (secondary)-0.8%[-1.2%, -0.6%]8
All (primary)-0.2%[-1.6%, 1.0%]142
  • These performance changes were anticipated, and the improvements outweigh the regressions.
  • Marking as triaged.

Rollup of 8 pull requests #122256 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.6%[0.2%, 1.6%]21
Regressions (secondary)0.6%[0.3%, 1.6%]7
Improvements (primary)-0.7%[-1.0%, -0.5%]3
Improvements (secondary)--0
All (primary)0.4%[-1.0%, 1.6%]24
  • Nadrieril has isolated this to the rolled up PR #120504 and has reported it on that PR.
  • (already marked as triaged)
  • It might be addressed by follow-up PR #122396 (which is undergoing evaluation now).

Distinguish between library and lang UB in assert_unsafe_precondition #121662 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)1.1%[0.3%, 1.7%]4
Regressions (secondary)1.1%[1.1%, 1.1%]1
Improvements (primary)-0.9%[-1.7%, -0.4%]4
Improvements (secondary)-0.2%[-0.2%, -0.2%]1
All (primary)0.1%[-1.7%, 1.7%]8
  • primary regressions are to cranelift-codegen opt-full (1.74%), cargo opt-full (1.33%), clap opt-full (1.18%), and exa debug-incr-unchanged (0.28%).
  • the rust-timer run was “only” expected to regress 5 benchmarks, largely a different set, by a mean of 0.5%, not the 1.1% reported above.
  • not marking as triaged

Stop using LLVM struct types for byval/sret #122050 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)1.9%[0.5%, 3.3%]2
Improvements (primary)-2.1%[-2.4%, -1.9%]2
Improvements (secondary)--0
All (primary)-2.1%[-2.4%, -1.9%]2
  • already marked as triaged as the reported regressions are clearly spurious based on the performance graph

Nominated Issues

T-compiler

  • “Stabilize extended_varargs_abi_support” rust#116161
    • Waiting on T-lang (discussed last week)
    • Will probably remove T-compiler nomination until then …

RFC

  • No I-compiler-nominated RFCs this time.

Oldest PRs waiting for review

T-compiler

  • “[AIX] Remove AixLinker’s debuginfo() implementation” rust#117118 (last review activity: 4 months ago)
    • @Wesley Wiser can you rubber stamp this one? It comes from the AIX target maintainer, seems fine (last week comment)
  • “tidy watcher” rust#114209
    • cc: @Wesley Wiser (or re-roll?)
  • “Require type_map::stub callers to supply file information” rust#104342 (last review activity: 2 months ago)
    • cc: @Wesley Wiser
  • “Fix gce ICE when encountering ill-formed consts” rust#119060 (last review activity: 2 months ago)
    • cc: @Michael Goulet (compiler-errors)
  • “dump-ice-to-disk/check.sh: convert needless bashism in a /bin/sh script.” rust#119992 (last review activity: about 58 days ago)
    • cc: @nils (Nilstrieb)

Next week’s WG checkins

  • @_WG-rustc-dev-guide by @Santiago Pastorino and @Yuki Okushi|217081
  • Impl Trait initiative by @oli

Next meetings’ agenda draft: hackmd link