T Compiler Meeting Agenda 2024 09 26

T-compiler Meeting Agenda 2024-09-26

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).

Other WG meetings

MCPs/FCPs

  • New MCPs (take a look, see if you like them!)
  • Old MCPs (not seconded, take a look)
    • “Add hygiene attributes to compile expanded source code” compiler-team#692 (Zulip) (last review activity: about 7 days ago)
    • “Target families for executable format” compiler-team#716 (Zulip) (last review activity: 7 months ago)
    • “Add Hotpatch flag” compiler-team#745 (Zulip) (last review activity: about 20 days ago)
    • “Opt-in flag for absolute paths in diagnostics” compiler-team#770 (Zulip) (last review activity: about 41 days ago)
    • “Add evex512 target feature for AVX10” compiler-team#778 (Zulip) (last review activity: about 20 days ago)
  • Pending FCP requests (check your boxes!)
    • “Add a new --orchestrator-id flag to rustc” compiler-team#635 (Zulip)
    • “sanitizers: Stabilize AddressSanitizer and LeakSanitizer for the Tier 1 targets” rust#123617
    • “Add --print host-triple to print host target triple” rust#125579
    • “Check ABI target compatibility for function pointers” rust#128784
    • “make unsupported_calling_conventions a hard error” rust#129935
  • Things in FCP (make sure you’re good with it)
  • Accepted MCPs
  • MCPs blocked on unresolved concerns
  • Finalized FCPs (disposition merge)
    • None
  • Other teams finalized FCPs
    • “Check WF of source type’s signature on fn pointer cast” rust#129021
    • “Relate receiver invariantly in method probe for Mode::Pathrust#129073
    • “Stabilize &mut (and *mut) as well as &Cell (and *const Cell) in const” rust#129195
    • “Make destructors on extern "C" frames to be executed” rust#129582
    • “stabilize const_extern_fnrust#129753

WG checkins

None

Backport nominations

T-compiler beta / T-compiler stable

  • :beta:“Use equality when relating formal and expected type in arg checking” rust#129317
    • Authored by compiler-errors
    • Fixes #129286 a P-medium regression on nightly (regressed in #129059)
  • :beta:“Don’t call extern_crate when local crate name is the same as a dependency and we have a trait error” rust#130275
    • Authored by compiler-errors
    • Fixes P-high (more likely P-critical) beta regression #130573 and #130272 and #129184 (all leading to the same root cause)
  • 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

  • “Unsoundness: Patterns in function parameters are not checked for union access” rust#130528
    • Fixed by @Michael Goulet (compiler-errors) in #130531 (beta accepted last week, open just to track the backport)
  • “Panic while compiling http-body v0.4.6 with rustc 1.83.0-nightly (7042c269c 2024-09-23)” rust#130769
    • regressed on recent nightly and already fixed by @_Quentin Dian (dianqk) in #130786 should fix this

T-types

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

P-high regressions

P-high beta regressions

  • “regression: overflow evaluating the requirement” rust#128887
    • Already fixed in #129073, just needs a regression test
  • “regression: ICE is not supported for this key” rust#130573
    • Solved by #130275, beta-nominated

Unassigned P-high nightly regressions

  • No unassigned P-high nightly regressions this time.

Performance logs

triage logs 2024-09-23

Not too much happened this week. Most regressions of note were readily justified as removing sources of unpredictable/inconsistent behavior from code-generation. There was one notable improvement, from PR #130561: avoiding redoing a redundant normalization of the param-env ended up improving compile times for 93 primary benchmarks by -1.0% on average.

Triage done by @pnkfelix. Revision range: 170d6cb8..749f80ab Revision range: 506f22b4..4cadeda9

Summary:

Note: Due to issues in how the perf website generates its output and fails to deal with certain git histories, we have two different base summaries.

Base Revision range: 170d6cb8..749f80ab

(instructions:u) mean range count
Regressions (primary) 0.7% [0.2%, 2.5%] 8
Regressions (secondary) 1.2% [0.1%, 3.2%] 4
Improvements (primary) -0.3% [-0.4%, -0.3%] 9
Improvements (secondary) -0.9% [-1.7%, -0.2%] 23
All (primary) 0.2% [-0.4%, 2.5%] 17

Base Revision range: 506f22b4..4cadeda9

(instructions:u) mean range count
Regressions (primary) 2.7% [0.1%, 6.7%] 16
Regressions (secondary) 0.2% [0.2%, 0.2%] 6
Improvements (primary) -1.1% [-5.1%, -0.2%] 80
Improvements (secondary) -2.4% [-5.2%, -0.2%] 54
All (primary) -0.5% [-5.1%, 6.7%] 96

Total:

2 Regression, 2 Improvements, 7 Mixed; 4 of them in rollups 62 artifact comparisons made in total

Regressions

Remove semi-nondeterminism of DefPathHash ordering from inliner #130455 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.8% [0.3%, 2.5%] 5
Regressions (secondary) 0.6% [0.1%, 1.0%] 3
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.8% [0.3%, 2.5%] 5
  • primary regressions are all to opt-full for hyper (-2.54%), clap (-0.69%), html5ever (-0.36%), image (-0.31%), cargo (-0.27%)
  • from detailed results on hyper, looks like mir_callgraph_reachable doubled in instr-count; if I’m reading the table right, it seems like it is being called nearly twice as often as before? But this outcome seems like it might be expected, since this PR is deliberately removing some code that would bail out early from that call.
  • it looks like @Ben Kimock (Saethlin) had already attempted to mark this as triaged (due to it being acceptable overhead for the benefit of better de

Rollup of 6 pull requests #130649 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 0.3% [0.1%, 0.4%] 13
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) - - 0
  • This was flagged solely because of the number of secondary benchmarks that were affected.
  • Marking as triaged.

Improvements

[perf] skip normalizing param env if it is already normalized #130561 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -1.0% [-5.5%, -0.1%] 93
Improvements (secondary) -2.7% [-6.5%, -0.2%] 59
All (primary) -1.0% [-5.5%, -0.1%] 93
  • This is awesome. :)

Rollup of 7 pull requests #130768 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) - - 0
Improvements (secondary) -0.3% [-0.7%, -0.1%] 14
All (primary) - - 0
  • This was flagged because of the number of secondary benchmarks that were affected.

Mixed

read_volatile __rust_no_alloc_shim_is_unstable in alloc_zeroed #130497 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.5%, 0.5%] 1
Regressions (secondary) - - 0
Improvements (primary) -0.5% [-0.7%, -0.3%] 2
Improvements (secondary) - - 0
All (primary) -0.2% [-0.7%, 0.5%] 3
  • cranelift-codegen opt-full-llvm regressed by -0.48%.
  • marking as triaged; this is bringing the two allocator paths into consistency with eachother (the question of whether we want these volatile reads at all, and how to otherwise achieve their effect on the end-to-end developer experience, is an entirely separate question).

Rollup of 9 pull requests #130534 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.3% [0.2%, 0.3%] 3
Regressions (secondary) - - 0
Improvements (primary) - - 0
Improvements (secondary) -1.1% [-1.2%, -0.6%] 4
All (primary) 0.3% [0.2%, 0.3%] 3
  • syn regressed on three variant profiles: check incr-unchanged (-0.28%), check incr-patched:println (-0.27%), and debug incr-unchanged (-0.23%)
  • detailed perf results indicates that incr_comp_persist_dep_graph went from 0.002 seconds to 0.14 seconds; a delta that accounts for the vast bulk of the difference here…
  • but from examining the PR’s that follow, it doesn’t seem like that result persists into the future. (Also, I’m realizing that the detailed results are presenting seconds, not instruction-counts, and therefore they are probably wildly unstable between runs…)
  • marking as triaged; this isn’t worth investigating further.

Support char::encode_utf8 in const scenarios. #130511 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.3% [0.3%, 0.4%] 3
Regressions (secondary) - - 0
Improvements (primary) -0.4% [-0.5%, -0.3%] 2
Improvements (secondary) - - 0
All (primary) 0.0% [-0.5%, 0.4%] 5
  • primary regressions to opt-full on clap (-0.38%), image (-0.33%), and webrender (-0.31%).
  • from inspection, it seems like this may just be a natural (and small, acceptable) artifact of the changes to which things get const-eval’ed.
  • marking as triaged

Get rid of niche selection’s dependence on fields’s order #130508 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.2% [0.2%, 0.2%] 1
Regressions (secondary) 0.5% [0.2%, 0.9%] 3
Improvements (primary) -0.6% [-0.6%, -0.6%] 1
Improvements (secondary) - - 0
All (primary) -0.2% [-0.6%, 0.2%] 2
  • primary regression to typenum doc-full (-0.23%)
  • marking as triaged; this is a very small (and probably semi-random) cost for a clear win IMO in terms of overall consistency of behavior.

Rollup of 7 pull requests #130631 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.6% [0.6%, 0.6%] 1
Regressions (secondary) 0.3% [0.2%, 0.4%] 5
Improvements (primary) -0.3% [-0.3%, -0.3%] 1
Improvements (secondary) - - 0
All (primary) 0.1% [-0.3%, 0.6%] 2
  • sole primary regression is to cargo opt-full by 0.6%.
  • no obvious smoking guns in the PR list for this rollup.
  • marking as triaged as “not being worth further investigation”

add unqualified_local_imports lint #125645 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 1.5% [0.4%, 2.0%] 11
Improvements (primary) - - 0
Improvements (secondary) -0.3% [-0.4%, -0.2%] 6
All (primary) - - 0
  • There was some surprise here for the PR author because this was an allow-by-default lint and so they didn’t expect it to have any actual perf impact, because they assumed that allowed-linted could be skipped (which is not actually the way they work today, but there are PR’s in process that try to get that effect).
  • In any case, this PR was flagged solely because of the number of secondary benchmarks that were affected.
  • Marking as triaged.

Rework non_local_definitions lint to only use a syntactic heuristic #127117 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 3.3% [0.6%, 6.6%] 13
Regressions (secondary) 0.3% [0.1%, 0.4%] 3
Improvements (primary) -0.7% [-1.0%, -0.2%] 8
Improvements (secondary) -0.3% [-0.3%, -0.3%] 3
All (primary) 1.8% [-1.0%, 6.6%] 21
  • as noted by Urgau: “the version of diesel used by rustc-perf is too old and use named const AA items instead of un-named const _ items, making the lint trigger many times (more than 150 times last time). We ruled in the past that given the regression where located in one benchmark only, and only a pathological case that the regressions where fine.”
  • already marked as triaged.

Nominated Issues

T-compiler

  • No I-compiler-nominated issues this time.

RFC

  • No I-compiler-nominated RFCs this time.

Oldest PRs waiting for review

T-compiler

  • “Consistently use the highest bit of vector masks when converting to i1 vectors” rust#104693 (last review activity: 14 months ago)
    • cc @Wesley Wiser (self-assigned last year, PR has bounced a bit looking for a reviewer)
  • “add error message for c# style named arguments” rust#118733 (last review activity: 9 months ago)
    • cc @Esteban Küber
  • “Fix bug where option_env! would return None when env var is present but not valid Unicode” rust#122670
    • cc: @lcnr (or reroll?)
  • “Issue 83060 fix” rust#119798
    • Can anyone assist the author? :) @Michael Goulet (compiler-errors) can you reply to the author’s comment?

Next meeting’s agenda draft: hackmd link