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
- Types Team: ITE (Impl Trait Everywhere) Triage time:2024-09-26T22:00:00+01:00
- wg-rust-analyzer steering meeting on Zulip (https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Steering.20meeting) time:2024-09-27T17:00:00+01:00
- wg-macros triage meeting Zulip (https://rust-lang.zulipchat.com/#narrow/stream/410876-wg-macros.2Ftriage) time:2024-09-27T18:00:00+01:00
MCPs/FCPs
- New MCPs (take a look, see if you like them!)
- “Make the test cfg a userspace check-cfg” compiler-team#785 (Zulip)
- 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
- “Add a new
- Things in FCP (make sure you’re good with it)
- “-Z default-visibility option” compiler-team#782 (Zulip)
- “Support tests to use
minicore
std/core prelude stubs” compiler-team#786 (Zulip)
- Accepted MCPs
- “Test rule annotations” compiler-team#783 (Zulip)
- “Don’t track
--emit=
options as part of crate SVH” compiler-team#769 (Zulip) - “
--hint-msrv=version
option so the compiler can take MSRV into account when linting” compiler-team#772 (Zulip) - “mark some target features as ‘forbidden’ so they cannot be (un)set with -Ctarget-feature” compiler-team#780 (Zulip)
- “New Tier-3 target for OpenHarmony on LoongArch64” compiler-team#784 (Zulip)
- MCPs blocked on unresolved concerns
- “Retire the mailing list and make all decisions on zulip” compiler-team#649 (Zulip)
- concern: automatic-sync
- “Retire the mailing list and make all decisions on zulip” compiler-team#649 (Zulip)
- 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::Path
” rust#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_fn
” rust#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.
- No beta nominations for
T-types
this time. - No stable nominations for
T-types
this time.
PRs S-waiting-on-team
Issues of Note
Short Summary
- 2 T-compiler P-critical issues
- 68 T-compiler P-high issues
- 0 P-critical, 3 P-high, 3 P-medium, 0 P-low regression-from-stable-to-beta
- 1 P-critical, 0 P-high, 6 P-medium, 3 P-low regression-from-stable-to-nightly
- 1 P-critical, 36 P-high, 100 P-medium, 17 P-low regression-from-stable-to-stable
P-critical
- “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
- No
P-critical
issues forT-types
this time.
P-high 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
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 namedconst AA
items instead of un-namedconst _
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
- No I-compiler-nominated issues this time.
- No I-compiler-nominated RFCs this time.
Oldest PRs waiting for review
- “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 returnNone
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