T Compiler Meeting Agenda 2024 05 30

T-compiler Meeting Agenda 2024-05-30

Announcements

  • Friday [steering meeting]: Project Goals by @_nikomatsakis
  • 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).

MCPs/FCPs

WG checkins

  • @_WG-async by @nikomatsakis and @tmandry

    Checkin text

  • @_WG-diagnostics by @Esteban Küber and @oli

    Checkin text

Backport nominations

T-compiler beta / T-compiler stable

  • :beta: “Fix ICE in non-operand aggregate_raw_ptr intrinsic codegen” rust#125184
    • Authored by scottmcm
    • Ben nominated:

      I would like this in beta so that the stage0 compiler does not ICE if it gets passed -Zmir-opt-level=0. Yes I can work around this issue without huge difficulty, but the diff here is so small.

  • :beta: “Revert problematic opaque type change” rust#125489
    • Authored by oli-obk
    • Fixes P-critical beta regression
  • :beta: “[BETA] revert leak check changes” rust#125629
    • Authored by lcnr
    • Fixes P-critical beta regression
  • :beta: “ast: Revert a breaking attribute visiting order change #125734”
    • Authored by petrochenkov
    • Fixes P-high beta regression
  • 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

  • -C target_cpu=cortex-a72 (and -target-cpu=native on Raspberry Pi) wrongly enables crypto features that are optional on Cortex-A72” rust#125033
    • Discussed extensively last week
  • “regression: trait bound not satisfied” rust#125194
    • Fix was beta nominated
  • “regression: type annotations needed” rust#125196
  • “regression: ambiguous outer attributes” rust#125199

T-types

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

P-high regressions

P-high beta regressions

  • “regression: cannot find macro in scope” rust#125201
    • Fix was beta nominated

Unassigned P-high nightly regressions

  • No unassigned P-high nightly regressions this time.

Performance logs

2024-05-27 Triage Log

A relatively quiet week, with few large changes, the largest driven by further increasing the scope of unsafe precondition checking.

Triage done by @simulacrum. Revision range: 1d0e4afd..a59072ec

Summary:

(instructions:u) mean range count
Regressions ❌ (primary) 1.6% [0.2%, 6.3%] 84
Regressions ❌ (secondary) 0.9% [0.1%, 3.8%] 66
Improvements ✅ (primary) -0.4% [-2.3%, -0.2%] 37
Improvements ✅ (secondary) -1.7% [-3.8%, -0.2%] 22
All ❌✅ (primary) 1.0% [-2.3%, 6.3%] 121

2 Regressions, 3 Improvements, 5 Mixed; 3 of them in rollups 51 artifact comparisons made in total

Regressions

Rewrite native thread-local storage #116123 (Comparison Link)

(instructions:u) mean range count
Regressions ❌ (primary) 0.4% [0.2%, 1.2%] 10
Regressions ❌ (secondary) 1.0% [0.7%, 1.6%] 9
Improvements ✅ (primary) - - 0
Improvements ✅ (secondary) - - 0
All ❌✅ (primary) 0.4% [0.2%, 1.2%] 10

Regressions deemed worth the overall change’s contents (see https://github.com/rust-lang/rust/pull/116123#issuecomment-2009408868). The TLS state is now stored in a single thread-local object rather than two, which should decrease costs of addressing it in general (modulo LLVM difficulties).

Rollup of 6 pull requests #125463 (Comparison Link)

(instructions:u) mean range count
Regressions ❌ (primary) 1.4% [1.1%, 2.0%] 8
Regressions ❌ (secondary) 1.0% [0.4%, 1.8%] 23
Improvements ✅ (primary) -0.5% [-0.5%, -0.5%] 1
Improvements ✅ (secondary) -0.3% [-0.3%, -0.3%] 1
All ❌✅ (primary) 1.2% [-0.5%, 2.0%] 9

Likely related to fixing an issue around LLD discovery, see https://github.com/rust-lang/rust/pull/125463#issuecomment-2129069901. Bugfix warrants spending a bit more time.

Improvements

Move the checks for Arguments constructors to inline const #125518 (Comparison Link)

(instructions:u) mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) - - 0
Improvements ✅ (primary) -0.5% [-0.7%, -0.3%] 5
Improvements ✅ (secondary) -0.5% [-0.9%, -0.4%] 7
All ❌✅ (primary) -0.5% [-0.7%, -0.3%] 5

Remove DefId from EarlyParamRegion #125468 (Comparison Link)

(instructions:u) mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) - - 0
Improvements ✅ (primary) -0.4% [-0.9%, -0.2%] 48
Improvements ✅ (secondary) -0.8% [-2.0%, -0.3%] 19
All ❌✅ (primary) -0.4% [-0.9%, -0.2%] 48

[perf] Delay the construction of early lint diag structs #125410 (Comparison Link)

(instructions:u) mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) - - 0
Improvements ✅ (primary) -0.5% [-0.7%, -0.3%] 12
Improvements ✅ (secondary) -2.0% [-2.8%, -1.2%] 13
All ❌✅ (primary) -0.5% [-0.7%, -0.3%] 12

Mixed

Follow-up fixes to report_return_mismatched_types #123812 (Comparison Link)

(instructions:u) mean range count
Regressions ❌ (primary) 1.1% [0.6%, 1.8%] 3
Regressions ❌ (secondary) 0.2% [0.1%, 0.2%] 6
Improvements ✅ (primary) - - 0
Improvements ✅ (secondary) -1.1% [-1.1%, -1.1%] 2
All ❌✅ (primary) 1.1% [0.6%, 1.8%] 3

Seems to be limited primarily to one scenario in regex, with lots of new metadata decoding. Asked for follow-up by PR author.

Make early lints translatable #124417 (Comparison Link)

(instructions:u) mean range count
Regressions ❌ (primary) 1.5% [0.2%, 6.3%] 66
Regressions ❌ (secondary) 0.4% [0.3%, 0.5%] 7
Improvements ✅ (primary) - - 0
Improvements ✅ (secondary) -0.7% [-1.1%, -0.6%] 12
All ❌✅ (primary) 1.5% [0.2%, 6.3%] 66

Regressions are partially mitigated by #125410 (see earlier in the report). Further follow-up is expected to investigate closing remaining gap (https://github.com/rust-lang/rust/pull/124417#issuecomment-2126056523).

Panic directly in Arguments::new* instead of recursing #117804 (Comparison Link)

(instructions:u) mean range count
Regressions ❌ (primary) 0.3% [0.3%, 0.3%] 2
Regressions ❌ (secondary) 0.4% [0.1%, 0.9%] 9
Improvements ✅ (primary) -0.5% [-0.5%, -0.5%] 1
Improvements ✅ (secondary) - - 0
All ❌✅ (primary) 0.0% [-0.5%, 0.3%] 3

Regressions are likely to be inliner noise, not meaningful changes.

Rollup of 7 pull requests #125456 (Comparison Link)

(instructions:u) mean range count
Regressions ❌ (primary) - - 0
Regressions ❌ (secondary) 0.3% [0.2%, 0.4%] 8
Improvements ✅ (primary) -0.3% [-0.4%, -0.3%] 2
Improvements ✅ (secondary) -0.5% [-0.9%, -0.4%] 7
All ❌✅ (primary) -0.3% [-0.4%, -0.3%] 2

Seems plausible that the regression is due to #124389 (since it affected derive macros). But also seems not worth further investigation given it’s a secondary benchmark and minimal impact. Marked as triaged.

Add assert_unsafe_precondition to unchecked_{add,sub,neg,mul,shl,shr} methods #121571 (Comparison Link)

(instructions:u) mean range count
Regressions ❌ (primary) 1.5% [0.2%, 3.4%] 27
Regressions ❌ (secondary) 1.8% [0.4%, 3.8%] 5
Improvements ✅ (primary) -0.9% [-2.5%, -0.3%] 5
Improvements ✅ (secondary) - - 0
All ❌✅ (primary) 1.1% [-2.5%, 3.4%] 32

Regressions are likely expected as we’re lowering more code that’s late-removed.

Nominated Issues

T-compiler

  • No I-compiler-nominated issues this time.

RFC

  • No I-compiler-nominated RFCs this time.

Oldest PRs waiting for review

None this week

Next meetings' agenda draft: hackmd link