T Compiler Meeting Agenda 2024 08 15

T-compiler Meeting Agenda 2024-08-15

Announcements

Other WG meetings

MCPs/FCPs

WG checkins

None

Backport nominations

T-compiler beta / T-compiler stable

  • :beta:“Do not apply #[do_not_recommend] if the feature flag is not set” rust#128674
    • Authored by weiznich
    • Adds additional checks for the feature flag (as apparently it is possible to use this on a beta compiler without feature flags).
    • Fixes an issue reported on Bevy (link)
    • Michael suggests backporting either this or #128912 (comment)
  • :beta:“Store do_not_recommend-ness in impl header” rust#128912
    • Authored by compiler-errors
    • Alternate approach to #128674:

    This only affects the error path, but it’s still strange and probably not something we want users to rely on implicitly. It’s less flexible, but also less invasive. Hopefully it’s also performant.

  • :beta:“Revert #125915 on beta” rust#128760
    • Authored by BoxyUwU
    • Fixes #128016 (P-critical), reverts #125915:

    This reverts commit #125915 on beta preventing #128016 from reaching stable. This breaks the argon2rs crate which causes a fair amount of downstream breakage of random projects. I don’t think its necessarily a huge deal but this was an incredibly clean revert and it does prevent breakage of code that used to work so I think this is worth it.

  • :beta:“derive(SmartPointer): register helper attributes” rust#128925
    • Authored by dingxiangfei2009
    • Fixes #128888, P-high beta regression found by a crater run
  • :beta:“Fix bug in Parser::look_ahead.” rust#128994
    • Authored by nnethercote
    • Fixes #128895, P-high beta regression found by a crater run
  • 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

  • “ICE: adding a def'n for node-id NodeId(18) and def kind AnonConst but a previous def'n exists” rust#128016
    • Reverted in beta by #128760
    • Will be fixed by #128844
  • “regression: adding a def’n for node-id NodeId(597) and def kind AnonConst but a previous def’n exists” rust#128901
    • Can’t find a discussion but since it’s a duplicate of #128016, assumed it will be fixed by #128760

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
    • Breaking change introduced in #126128, which could be accepted (comment)
  • “regression: expected one of !, (, +, ::, <, >, or as, found :” rust#128895
    • fixed by #128994 (nominated for beta backport)

Unassigned P-high nightly regressions

  • “Generated WebAssembly unexpectedly requires reference types” rust#128475
    • (discussed last week)
    • Alex Chrichton authored #128511

Performance logs

triage logs for 2024-08-13

A big week for compiler performance brought on mostly by statically linking the std library into rustc_driver instead of dynamic linking. This overshadows all other improvements and regressions that were seen this week.

Triage done by @rylev. Revision range: 8c7e0e16..9cb1998e

Summary:

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -1.3% [-2.9%, -0.2%] 217
Improvements (secondary) -1.4% [-4.9%, -0.2%] 196
All (primary) -1.3% [-2.9%, -0.2%] 217

2 Regressions, 2 Improvements, 2 Mixed; 1 of them in rollups 35 artifact comparisons made in total

Regressions

Rollup of 7 pull requests #128768 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.4% [0.4%, 0.4%] 1
Regressions (secondary) 0.3% [0.2%, 0.4%] 11
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.4% [0.4%, 0.4%] 1
  • All docs regressions caused by #128417 which is simply just documenting more.

Stabilize min_exhaustive_patterns #122792 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.8% [0.3%, 1.5%] 13
Regressions (secondary) - - 0
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.8% [0.3%, 1.5%] 13
  • Seems this has a negative impact on the performance of coherence checking which I imagine is expected.
  • Confirming with the author/reviewer.

Improvements

Cache supertrait outlives of impl header for soundness check #128746 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.2% [0.2%, 0.2%] 1
Regressions (secondary) 0.0% [0.0%, 0.0%] 1
Improvements (primary) -0.5% [-1.3%, -0.2%] 69
Improvements (secondary) -1.8% [-3.8%, -0.2%] 30
All (primary) -0.5% [-1.3%, 0.2%] 70

Link std statically in rustc_driver #122362 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -1.0% [-2.0%, -0.2%] 229
Improvements (secondary) -1.0% [-2.0%, -0.2%] 222
All (primary) -1.0% [-2.0%, -0.2%] 229

Mixed

Only walk ribs to collect possibly shadowed params if we are adding params in our new rib #128550 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 2.2% [2.2%, 2.2%] 1
Improvements (primary) -0.6% [-2.0%, -0.2%] 31
Improvements (secondary) -0.3% [-0.3%, -0.2%] 6
All (primary) -0.6% [-2.0%, -0.2%] 31
  • Regression seems to be noise.

Apply “polymorphization at home” to RawVec #126793 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.2%, 1.6%] 98
Regressions (secondary) 0.4% [0.1%, 1.2%] 125
Improvements (primary) -1.0% [-3.5%, -0.2%] 51
Improvements (secondary) -1.3% [-2.9%, -0.3%] 13
All (primary) -0.0% [-3.5%, 1.6%] 149
  • “there’s a bunch of instruction regressions – improvements too, but fewer of those – but looking at cycles, wall time, bootstrap, and binary size it looks consistently great. And the overall approach is good, so there should be space to get smaller improvements with tweaks to things like mir inlining”

Nominated Issues

T-compiler

  • “ill-typed unused FFI declarations can cause UB” rust#46188
    • Discussed last week but didn’t finish (comment)
    • Discussion can resume from here about landing #128247 as a mitigation
    • Should be a T-lang discussion?
  • “[crater] Make missing_fragment_specifier an unconditional error” rust#128425
    • Nominated by @Trevor Gross (comment)
    • Asks to evaluate the fallout of making this an error (comment)
  • “type_id is not sufficiently collision-resistant” rust#129014
    • authored and nominated by @RalfJ
    • See opening comment
  • “Tracking Issue for the C-cmse-nonsecure-call ABI” rust#81391
    • nominated by @Jieyou Xu (comment):
    • also nominated for T-lang
    • Does this need an MCP/FCP for stabilization, or does this need further design?
    • Does this need a joint T-compiler/T-lang FCP?
    • We should help the people working on cmse-related things to find knowledgeable reviewers / domain experts who can help reviewing the changes, or otherwise provide advice on how to split related PRs for easier reviewing.
    • Are cmse-related efforts being tracked anywhere?
  • “[DRAFT] #[contracts::requires(…)]” rust#128045
    • nominated by @Jieyou Xu (comment): looking for a reviewer familiar with attribute handling / HIR/MIR changes
    • Felix left a comment: this is no longer “ready for review” – the review feedback has made it clear that this should not land as-is
    • The current challenge seems to be that existing attribute handling only supports a “single path segment”, i.e. #[contracts(<inner_expr>)] but not #[contracts::requires(<inner_expr>)]. We also can’t trivially reuse existing register_attr! or visitor because they impose strict limits on the <inner_expr> and require it to be very primitive (like literals or identifiers or simple lists) but does not support arbitrary expressions like x > 0. To support the arbitrary inner expression (in that it may not necessarily be a valid rust expression), it may require relaxing that restriction, but it may raise further parsing/grammar questions. It’s not clear what’s the best path forward here.

RFC

  • No I-compiler-nominated RFCs this time.

Oldest PRs waiting for review

Skipping this week

Next week’s WG checkins

None

Next meetings' agenda draft: hackmd link