T Compiler Meeting Agenda 2023 07 20

T-compiler Meeting Agenda 2023-07-20

Announcements

  • Proposal from @apiraino: removing T-rustdoc backport nominations from T-compiler meeting’s agenda, T-rustdoc now has their own meetings and can take on that. Is everybody happy with that?
  • 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).

Other WG meetings (calendar link)

MCPs/FCPs

  • New MCPs (take a look, see if you like them!)
    • “Expose default_hidden_visibility as a rustc command line option” compiler-team#656
  • Old MCPs (not seconded, take a look)
    • “Cell Broadband Engine SPU support” compiler-team#614 (last review activity: 3 months ago)
    • “Add support for Zephyr OS” compiler-team#629 (last review activity: about 28 days ago)
    • “Consistently use “region” terminology in later stages of the compiler” compiler-team#634 (last review activity: about 55 days ago)
    • “Add a new --build-id flag to rustc” compiler-team#635 (last review activity: about 55 days ago)
    • “Simplify and improve explicitness of the check-cfg syntax” compiler-team#636 (last review activity: about 35 days ago)
    • “[MCP] proposing a macros working group” compiler-team#637 (last review activity: about 34 days ago)
    • “Add support for visionOS targets” compiler-team#642 (last review activity: about 20 days ago)
    • “Add illumos Tier3 targets” compiler-team#644 (last review activity: about 20 days ago)
    • “Migrate away from u32 as an offset/length type” compiler-team#647 (last review activity: about 13 days ago)
    • “New tier-3 targets for TEEOS” compiler-team#652 (last review activity: about 6 days ago)
  • Pending FCP requests (check your boxes!)
    • “Retire the mailing list and make all decisions on zulip” compiler-team#649
    • “Support overriding warnings level for a specific lint via command line” rust#113307
  • Things in FCP (make sure you’re good with it)
  • Accepted MCPs
    • No new accepted proposals this time.
  • Finalized FCPs (disposition merge)
    • No new finished FCP (disposition merge) this time.

WG checkins

  • @_WG-rls2.0 by @Lukas Wirth (last checkin)

    Since last time we’ve had a lot of MIR improvements from hkalbasi (we can actually interpret a few of the r-a tests in MIR form). Also notable was upgrading chalk, which should have fixed a bunch of long-standing issues and the start of a memory layout viewer.

  • @_WG-self-profile by @mw and @Wesley Wiser (last checkin)

    check-in text

Backport nominations

T-compiler stable / T-compiler beta

  • :beta: [1.72] “make MCP510 behavior opt-in to avoid conflicts between the CLI and target flavors” rust#113631
    • nominated by @lqd (comment)
    • Fixes #113597 (now in beta) and bypasses the new behavior introduced in #112910
  • :beta: [1.72] “allow opaques to be defined by trait queries, again” rust#113690
    • reverts #112963 and fixes #113689, nominated by @oli
  • No stable nominations for T-compiler this time.

:back: / :shrug: / :hand:

PRs S-waiting-on-team

T-compiler

Issues of Note

Short Summary

P-critical

T-compiler

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

T-types

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

T-rustdoc

  • No P-critical issues for T-rustdoc at 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 2023-07-18

A lot of spurious results in the regressions this week. However, we did see some real gains with PR #113609, with nearly 40 real-world benchmarks improving their check-build performance by >=1%.

Triage done by @pnkfelix. Revision range: 1d4f5aff..6b9236ed

Summary:

(instructions:u)meanrangecount
Regressions (primary)1.5%[0.6%, 3.0%]11
Regressions (secondary)1.4%[0.6%, 1.8%]11
Improvements (primary)-1.6%[-3.7%, -0.6%]46
Improvements (secondary)-1.9%[-4.2%, -0.4%]46
All (primary)-1.0%[-3.7%, 3.0%]57

5 Regressions, 5 Improvements, 5 Mixed; 2 of them in rollups 57 artifact comparisons made in total 30 Untriaged Pull Requests

Regressions

miri: protect Move() function arguments during the call #113569 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.8%[0.5%, 1.1%]13
Regressions (secondary)0.9%[0.4%, 1.4%]11
Improvements (primary)--0
Improvements (secondary)--0
All (primary)0.8%[0.5%, 1.1%]13
  • RalfJ is investigating; has potential fix up in PR #113630, …
  • … but its not totally certain that PR is a real fix (i.e. the regression may already have been masked or otherwise resolved independently).
  • But meanwhile, I am hypothesizing that the regression reported here is spurious (discussion)
  • marking as triaged

Ignore flaky clippy tests. #113621 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)1.0%[0.6%, 1.2%]7
Regressions (secondary)--0
Improvements (primary)--0
Improvements (secondary)--0
All (primary)1.0%[0.6%, 1.2%]7
  • already marked as triaged (its noise)

Rollup of 6 pull requests #113673 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)1.2%[1.2%, 1.2%]1
Regressions (secondary)--0
Improvements (primary)--0
Improvements (secondary)--0
All (primary)1.2%[1.2%, 1.2%]1
  • addressed by PR #113697, already marked as triaged.

Add even more GHA log groups #113514 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)2.9%[2.7%, 3.0%]6
Improvements (primary)--0
Improvements (secondary)--0
All (primary)--0
  • PR author says “this doesn’t affect how the compiler was built at all, the perf regression must be spurious.”
  • I agree, marking as triaged

Rollup of 3 pull requests #113738 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.7%[0.7%, 0.7%]1
Regressions (secondary)3.1%[2.7%, 3.5%]6
Improvements (primary)--0
Improvements (secondary)--0
All (primary)0.7%[0.7%, 0.7%]1

Improvements

Rewrite UnDerefer, again #113316 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)1.2%[1.2%, 1.2%]1
Regressions (secondary)--0
Improvements (primary)-0.9%[-1.7%, -0.4%]14
Improvements (secondary)-1.2%[-2.0%, -0.2%]21
All (primary)-0.8%[-1.7%, 1.2%]15

(re-)tighten sourceinfo span of adjustments in MIR #112945 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)--0
Improvements (primary)-0.7%[-1.1%, -0.4%]18
Improvements (secondary)-0.5%[-0.6%, -0.5%]5
All (primary)-0.7%[-1.1%, -0.4%]18

Bump bootstrap to 1.72 beta #113637 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)--0
Improvements (primary)-0.9%[-1.2%, -0.6%]12
Improvements (secondary)--0
All (primary)-0.9%[-1.2%, -0.6%]12

Add a cache for maybe_lint_level_root_bounded #113609 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)--0
Improvements (primary)-1.4%[-3.4%, -0.5%]29
Improvements (secondary)-1.9%[-5.9%, -0.2%]33
All (primary)-1.4%[-3.4%, -0.5%]29

Remove LLVMRustCoverageHashCString #113430 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)--0
Improvements (primary)-0.7%[-0.7%, -0.7%]2
Improvements (secondary)-2.8%[-3.2%, -2.4%]6
All (primary)-0.7%[-0.7%, -0.7%]2

Mixed

Eliminate ZST allocations in Box and Vec #113113 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)1.7%[0.5%, 3.5%]3
Regressions (secondary)--0
Improvements (primary)-1.0%[-1.7%, -0.4%]2
Improvements (secondary)--0
All (primary)0.6%[-1.7%, 3.5%]5
  • regressions here were anticipated and unavoidable. This is a bug fix.
  • Marking as triaged.

Enable MIR reference propagation by default #109025 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)1.2%[0.5%, 2.5%]15
Regressions (secondary)0.8%[0.2%, 1.3%]7
Improvements (primary)-0.9%[-1.0%, -0.8%]3
Improvements (secondary)-0.6%[-1.1%, -0.4%]6
All (primary)0.8%[-1.0%, 2.5%]18
  • This is turning on a MIR pass at lower optimizations levels, so its expected that it would cause the compiler to do more work.
  • its clear from the perf runs on the PR itself that the PR author already put in much effort to make the pass faster than it had started out.
  • marking as triaged.

Resurrect: rustc_target: Add alignment to indirectly-passed by-value types, correcting the alignment of byval on x86 in the process. #112157 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)1.7%[1.5%, 2.0%]6
Regressions (secondary)--0
Improvements (primary)--0
Improvements (secondary)-2.8%[-2.9%, -2.8%]6
All (primary)1.7%[1.5%, 2.0%]6
  • The six primary regressions were all to variants of diesel (all of check/debug/opt) for variious full and incr-full scenarios.
  • It isn’t noise, there seems to be a clear cliff that starts with this PR when looking at the graph starting from 2023-0702.
  • not marking as triaged yet, but was tempted to do so, because this PR is a prerequiste for unlocking various memcpy optimizations added by pcwalton to LLVM

Remove unneeded handling for ExternalLocation::Unknown in rustdoc render context #113697 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)3.0%[2.5%, 3.4%]6
Improvements (primary)-1.1%[-1.1%, -1.1%]1
Improvements (secondary)--0
All (primary)-1.1%[-1.1%, -1.1%]1
  • this was mentioned up above when I was talking about PR #113673
  • the secondary regressions are all to ctfe-stress-5, which lqd says was noisy at this time.
  • marked as triaged.

Add support for allocators in Rc & Arc #89132 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.8%[0.8%, 0.8%]1
Regressions (secondary)--0
Improvements (primary)-1.3%[-1.3%, -1.3%]1
Improvements (secondary)-0.7%[-0.8%, -0.5%]4
All (primary)-0.3%[-1.3%, 0.8%]2
  • primary regression was to image-0.24.1 opt full by 0.79%
  • I think this is just noise. From the graph, it seems like image has unpredictably jumped up and down between two plateaus since PR #113113 (a PR discussed up above that changed low level allocation procotol code in Box and Vec, and thus might be expected to have some weird follow-on effects).
  • marked as triaged

Nominated Issues

T-compiler

  • “MSVC and rustc disagree on minimum stack alignment on x86 Windows” rust#112480
    • discussed last week (notes), needs further discussion.
  • ““Legacy” tier 2 targets have misplaced or absent maintainer docs” rust#113739
    • nominated by @Jubilee, Zulip discussion happening here
    • summary: some tier 2 compile targets are basically unsupported, we should probably refresh our list of support

RFC

  • No I-compiler-nominated RFCs this time.

Oldest PRs waiting for review

  • “Only unpack tupled args in inliner if we expect args to be unpacked” rust#110833 (last review activity: 2 months ago)
    • cc @cjgillot
  • “add a Callable trait that is implemented for unsafe functions, too” rust#107123
    • @Michael Goulet (compiler-errors) / @olireminder about triaging the crater report, thanks!
  • “Only run MaybeInitializedPlaces dataflow once to elaborate drops” rust#111555
    • cc: @Wesley Wiser
  • “Fix target_feature 1.1 should print the list of missing target features” rust#109710 (last review activity: about 41 days ago)
    • cc: @est31
  • “Tweak spans for self arg, fix borrow suggestion for signature mismatch” rust#112508 (last review activity: about 38 days ago)
    • cc: @Wesley Wiser

Next week’s WG checkins

  • Types team by @nikomatsakis and @Jack Huey
  • @_WG-mir-opt by @oli

Next week’s agenda draft: https://hackmd.io/IPpNv80sQAmD2ZTF6h56ng