T Compiler Meeting Agenda 2024 11 21

T-compiler Meeting Agenda 2024-11-21

Announcements

  • Next week stable 1.83 is out :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

  • New MCPs (take a look, see if you like them!)
    • No new proposals this time.
  • Old MCPs (stale MCP might be closed as per MCP procedure)
    • None at this time
  • Old MCPs (not seconded, take a look)
    • “Add hygiene attributes to compile expanded source code” compiler-team#692 (Zulip) (last review activity: 2 months ago)
    • “Add Hotpatch flag” compiler-team#745 (Zulip) (last review activity: about 40 days ago)
    • “Add evex512 target feature for AVX10” compiler-team#778 (Zulip) (last review activity: 2 months ago)
    • “Make the test cfg a userspace check-cfg” compiler-team#785 (Zulip) (last review activity: about 0 days ago)
    • “Policy change around adding new unstable flags” compiler-team#787 (Zulip) (last review activity: about 33 days ago)
    • “Normalize FileCheck directives” compiler-team#789 (Zulip) (last review activity: about 33 days ago)
    • “Relink, don’t rebuild” compiler-team#790 (Zulip) (last review activity: about 33 days ago)
    • “Create an avr-unknown-none target” compiler-team#800 (Zulip) (last review activity: about 11 days ago)
    • “Unstable flag to choose Emscripten unwinding abi” compiler-team#801 (Zulip) (last review activity: about 5 days ago)
  • Pending FCP requests (check your boxes!)
    • “sanitizers: Stabilize AddressSanitizer and LeakSanitizer for the Tier 1 targets” rust#123617
    • “Lint on combining #[no_mangle] and #[export_name]” rust#131558
  • Things in FCP (make sure you’re good with it)
  • Accepted MCPs
  • MCPs blocked on unresolved concerns
  • Finalized FCPs (disposition merge)
    • “Add --print host-tuple to print host target tuple” rust#125579
    • “Fix ICE when passing DefId-creating args to legacy_const_generics.” rust#130443
    • “Stabilize WebAssembly multivalue, reference-types, and tail-call target features” rust#131080
    • “make unsupported_calling_conventions a hard error” rust#129935
  • Other teams finalized FCPs
    • “Allow dropping dyn Trait principal” rust#126660
    • “Check elaborated projections from dyn don’t mention unconstrained late bound lifetimes” rust#130367
    • “Do not consider match/let/ref of place that evaluates to ! to diverge, disallow coercions from them too” rust#129392
    • “Finish stabilization of result_ffi_guarantees” rust#130628
    • “Fixup Windows verbatim paths when used with the include! macro” rust#125205
    • “Lint against getting pointers from immediately dropped temporaries” rust#128985
    • “Make deprecated_cfg_attr_crate_type_name a hard error” rust#129670
    • “Stabilize Arm64EC inline assembly” rust#131781
    • “Stabilize const ptr::write* and mem::replace” rust#130954
    • “Stabilize expr_2021 fragment specifier in all editions” rust#129972
    • “Stabilize s390x inline assembly” rust#131258
    • “Tracking issue for const_size_of_val and const_align_of_val” rust#46571
    • “atomics: allow atomic and non-atomic reads to race” rust#128778
    • “mark is_val_statically_known intrinsic as stably const-callable” rust#132449

Backport nominations

T-compiler beta / T-compiler stable

  • :beta: “Enforce that raw lifetimes must be valid raw identifiers” rust#132363
    • Authored and nominated by compiler-errors
    • Fix something we noticed (not an issue reported) see comment
    • Patch sibling to #132341 (work on #126452), approved for backport
    • FCP’d by t-lang, also in favor of a backport (comment)
  • :beta: “Add discriminators to DILocations when multiple functions are inlined into a single point.” rust#132613
    • Authored by khuey
    • we postponed the vote last week (comment) because we wanted to see how #132900 was (or wasn’t) related
    • So, yes, it was related (here more context) but #132900 is solved by a follow-up patch from @cuviper in #133194
  • :beta: “Update cdb annotations for some debuginfo tests with cdb 10.0.26100.2161” rust#133115
    • Authored by jieyouxu
    • Fixes some tests on windows target, CI image was updated CDB (MS debugger)
    • nominated by @_cuviper because this is probably affecting beta+stable 1.83 (comment)
  • :beta: “Drop debug info instead of panicking if we exceed LLVM’s capability to represent it” rust#133194
    • Authored by khuey
    • Fixes an ICE #132900
    • makes sense a backport of both this and #132613 (or neither, see comment)
  • 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

  • No P-high beta regressions this time.

Unassigned P-high nightly regressions

  • “error: the compiler unexpectedly panicked. this is a bug.” rust#132920
    • we have a smaller repro
    • seems originated in #128849 (comment)
    • P-high because the ICE completely shadows the error (but we can lower the priority)

Performance logs

triage logs 2024-11-19

We saw improvements to a large swath of benchmarks with the querification of MonoItem collection (PR #132566). There were also some PRs where we are willing to pay a compile-time cost for expected runtime benefit (PR #132870, PR #120370), or pay a small cost in the single-threaded case in exchange for a big parallel compilation win (PR #124780).

Triage done by @pnkfelix. Revision range: d4822c2d..7d40450b

Summary:

(instructions:u) mean range count
Regressions (primary) 1.1% [0.2%, 7.3%] 195
Regressions (secondary) 1.3% [0.1%, 5.0%] 194
Improvements (primary) -7.3% [-24.9%, -0.3%] 60
Improvements (secondary) -9.4% [-67.0%, -0.3%] 23
All (primary) -0.9% [-24.9%, 7.3%] 255

2 Regressions, 4 Improvements, 10 Mixed; 6 of them in rollups 47 artifact comparisons made in total 30 untriaged PRs

Regressions

Rollup of 5 pull requests #133039 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.3% [0.3%, 0.3%] 1
Regressions (secondary) 0.3% [0.2%, 0.4%] 13
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.3% [0.3%, 0.3%] 1
  • sole primary regression is to doc (full) on helloworld.
  • already triaged as being due to PR #133005 (and categorized as “improvements to rustdoc search that justify this cost”)

Rollup of 7 pull requests #133120 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.2% [0.1%, 0.3%] 25
Regressions (secondary) 0.2% [0.1%, 0.4%] 14
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.2% [0.1%, 0.3%] 25
  • primary regressions to {serde, libc, diesel, cargo, cranelift-codegen} (across multiple scenarios) and {html5ever, ripgrep, exa, syn, stm32f4, image, regex, webrender, hyper} (in isolated scenarios).
  • @nnethercote has hypothesized that PR #132134 may be responsible and is planning to investigate.
  • Not marking as triaged.

Improvements

move all mono-time checks into their own folder, and their own query #132843 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.2% [-0.2%, -0.2%] 4
Improvements (secondary) -0.4% [-0.4%, -0.4%] 1
All (primary) -0.2% [-0.2%, -0.2%] 4
  • (based on conversation in the PR itself, I think this “gain” is illusory and these benchmarks are just unstable)

Rollup of 4 pull requests #133047 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.2% [-0.4%, -0.0%] 2
Improvements (secondary) -0.2% [-0.4%, -0.1%] 4
All (primary) -0.2% [-0.4%, -0.0%] 2

Rollup of 5 pull requests #133086 (Comparison Link)

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

Rollup of 5 pull requests #133179 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.1% [-0.2%, -0.1%] 3
Improvements (secondary) -0.3% [-0.4%, -0.2%] 2
All (primary) -0.1% [-0.2%, -0.1%] 3

Mixed

Rollup of 7 pull requests #132919 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.4% [0.1%, 1.3%] 6
Regressions (secondary) 0.2% [0.1%, 0.2%] 2
Improvements (primary) -0.1% [-0.1%, -0.1%] 1
Improvements (secondary) -2.4% [-2.4%, -2.4%] 1
All (primary) 0.3% [-0.1%, 1.3%] 7
  • primary regressions are to doc scenarios; further investigation has indicated that PR #132144 is the root cause of them.
  • pnkfelix’s understanding is that this is known potential cost of progress when adding new traits (Receiver in this case) to stdlib.
  • marked as triaged

#[inline] integer parsing functions #132870 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.9% [0.1%, 5.1%] 12
Regressions (secondary) 1.0% [0.4%, 2.3%] 9
Improvements (primary) -0.4% [-0.6%, -0.2%] 3
Improvements (secondary) - - 0
All (primary) 0.6% [-0.6%, 5.1%] 15
  • main notable regressions were to regex opt-incr-patched (5%), cargo opt-full (1.14%), cranelift-codegen opt-full (0.83%) and ripgrep opt-full (0.74%); (everything else was <0.5% change).
  • this is incurring a compilation cost, especially on opt builds, because its inlining more stuff
  • pnkfelix assumes that the runtime benefit justifies this runtime cost.
  • marking as triaged

[perf] rustdoc: Perform less work when cleaning middle::ty parenthesized generic args #132886 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 1.0% [1.0%, 1.0%] 2
Regressions (secondary) 0.2% [0.2%, 0.2%] 1
Improvements (primary) - - 0
Improvements (secondary) -0.2% [-0.3%, -0.1%] 5
All (primary) 1.0% [1.0%, 1.0%] 2
  • already identified as measurement bias/noise and marked as triaged; thanks lqd!

Reland #132772: use download-rustc="if-unchanged" as a global default #132872 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 0.2% [0.2%, 0.3%] 2
Improvements (primary) -1.1% [-1.1%, -1.0%] 3
Improvements (secondary) -0.5% [-0.9%, -0.3%] 6
All (primary) -1.1% [-1.1%, -1.0%] 3
  • already identified as measurement bias/noise and marked as triaged; thanks lqd!

tweak attributes for const panic macro #132662 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.7% [0.5%, 0.9%] 2
Regressions (secondary) - - 0
Improvements (primary) -0.7% [-2.0%, -0.2%] 6
Improvements (secondary) - - 0
All (primary) -0.3% [-2.0%, 0.9%] 8
  • primary regressions are to image (0.92%) and webrender (0.50%) in opt-full scenarios.
  • it seems like this might be just noise from changes to inlining decisions, since it seems like both those cases are undoing artificial improvements introduced by PR #132870 (noted above)
  • marking as triaged

rustc_metadata: Preprocess search paths for better performance #132910 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.0% [0.0%, 0.0%] 1
Regressions (secondary) 2.2% [0.1%, 4.4%] 2
Improvements (primary) - - 0
Improvements (secondary) -0.2% [-0.2%, -0.2%] 2
All (primary) 0.0% [0.0%, 0.0%] 1
  • already marked by saethlin as triaged: “The reported regressions are extremely marginal, and the discussion above: #132910 (comment) contains instructions for how to demonstrate a quite significant benefit of this change.”

Querify MonoItem collection #132566 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 1.7% [0.3%, 6.6%] 10
Regressions (secondary) 1.7% [0.3%, 3.7%] 23
Improvements (primary) -6.3% [-25.1%, -0.1%] 73
Improvements (secondary) -9.9% [-67.2%, -0.3%] 22
All (primary) -5.4% [-25.1%, 6.6%] 83
  • massive improvements; many incremental benchmarks are improved by 5% to 20% (and even 25% in the case of ripgrep)
  • the regressions here (mostly to helloworld) were anticipated during review and are far outweighed by the very notable gains in compiler performance across a broad swath of benchmarks.
  • marking as triaged

Likely unlikely fix #120370 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.9% [0.1%, 2.8%] 4
Regressions (secondary) 0.8% [0.2%, 1.7%] 18
Improvements (primary) -0.4% [-1.9%, -0.1%] 21
Improvements (secondary) -0.3% [-0.4%, -0.1%] 11
All (primary) -0.2% [-1.9%, 2.8%] 25
  • primary regressions on html5ever opt-full (2.78%), image opt-incr-patched (0.35%) (and stm32f4 doc-full (0.11%) but that’s probably noise)
  • already triaged by saethlin; “some change is expected because these intrinsics used to do nothing” (i.e. they were broken before and are now believed to be fixed). Also clearly the benefits on the primary benchmarks is outweighing the limited primary regressions.

Use TypingMode throughout the compiler instead of ParamEnv #132460 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.2% [0.1%, 0.4%] 8
Regressions (secondary) 0.9% [0.2%, 2.7%] 6
Improvements (primary) -0.3% [-0.3%, -0.3%] 1
Improvements (secondary) -0.8% [-0.9%, -0.7%] 5
All (primary) 0.1% [-0.3%, 0.4%] 9
  • primary regressions to unicode-normalization profiles and (to a lesser extent) diesel, stm32f4, and cargo.
  • it seems like this regression was to some extent anticipated, at least from the perf run that was done while the PR was in development.
  • not formally marking as triaged yet; instead, left note saying that I’m inclined to mark as triage if either the PR author or the PR reviewer agree that this “regression”, such as it is, was anticipated.

Improve VecCache under parallel frontend #124780 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 1.0% [0.2%, 3.3%] 214
Regressions (secondary) 1.0% [0.2%, 2.9%] 172
Improvements (primary) - - 0
Improvements (secondary) -1.9% [-3.0%, -0.4%] 8
All (primary) 1.0% [0.2%, 3.3%] 214
  • left a note pointing out that there was a pretty serious process deviation, in that this was marked with rustc-perf-triaged for a different commit (maybe a different commit series?), months before this had gotten review or even landed…
  • but having said that, I looked over the data and it seems like the label remains justified, so I have left the label in place.

Nominated Issues

T-compiler

  • “Renaming targets might be a breaking change” rust#133030

    Does T-compiler want to amend the target policy docs to better reflect what compiler stability guarantees are made and what is explicitly not covered by compiler stability guarantees, as opposed to library API stability guarantees and language stability guarantees.

    • And:

    Are there any difference in stability guarantees (or the explicit lack thereof) for different-tiered compiler targets.

RFC

  • No I-compiler-nominated RFCs this time.

Oldest PRs waiting for review

None this week

Next meetings' agenda draft: hackmd link