T Compiler Meeting Agenda 2023 11 30

T-compiler Meeting Agenda 2023-11-30

Announcements

  • RFC: improve the onboarding experience of t-compiler + t-compiler-contributor (Zulip thread):
    • HackMD document: https://hackmd.io/Cp0Ktm2KQeS4TNmFn2UwWQ
    • @Santiago Pastorino @lqd and @apiraino are planning to work a bit on the rustc-dev + forge documentation as well
    • 👉 Feedback and wishlists around this topic are welcome!
  • 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 (calendar link)

MCPs/FCPs

WG checkins

Backport nominations

FIY Some stable backports were preemptively approved last week (Zulip thread):

T-compiler stable / T-compiler beta

  • :beta: 1.75 “Build pre-coroutine-transform coroutine body on error” rust#117686
    • Fixes an ICE #117670
    • nominated by Wesley (comment) because people are already tripping on this
  • :beta: 1.75 “ConstProp: Correctly remove const if unknown value assigned to it.” rust#118426
    • thanks @Alona Enraght-Moony !
    • Fixes rust#118328, P-high regression of a fuzzer-generated unsoundness
  • :beta: 1.75 “Dispose llvm::TargetMachines prior to llvm::Context being disposed” rust#118464
    • Fixes #118462 (thanks @Wesley Wiser ), a P-high UB on ARM (Windows only)
  • :stable: “Fix coroutine validation for mixed panic strategy” rust#118422
    • Fixes #116953, a P-high stable regression likely caused by a recent regression by MIR drop tracking (context)
    • Nominated by @_apiraino (comment), more in case a 1.74.1 dot release discussion starts (see other stable accepted backports)

PRs S-waiting-on-team

T-compiler

  • “[RFC 3086] Attempt to try to resolve blocking concerns” rust#117050
    • T-lang approved without FCP (comment)
    • anything else blocking? Is T-compiler still interested here?
  • “Remap paths from other crates” rust#117836
    • T-compiler mentioned to have a look (comment) and figure out if there are ramifications
  • “guarantee that char and u32 are ABI-compatible” rust#118032
    • T-lang discussed (comment), opened FCP process
  • Other issues in progress or waiting on other teams

Issues of Note

Short Summary

P-critical

T-compiler

  • “Miscompilation of Bevy (and some wgpu) apps resulting in segfault on macOS.” rust#117902
    • Actually will be fixed by fixing the wgpu crate (comment and following)
  • “1.74 causes an internal compiler error: broken MIR in Item” rust#117976
    • stable Regression is causing the project tantivy to fail compiling on 1.74. Reported problems also from a Asahi Linux kernel driver (comment)
    • @lqd investigated and reports that it doesn’t reproduce anymore on nightly+beta (comment). We probably just missed a backport.

T-types

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

P-high regressions

P-high beta regressions

  • “regression: new resolution failures in 1.74” rust#117056
    • @_apiraino thinks these are mostly fixed (anything missing here?)
  • “Problem running a method on the output of a function that returns an associated type (“missing optimized MIR”)” rust#117997
    • Will be fixed by #117076 (comment), already merged

Performance logs

triage logs for 2023-11-28

A good week, despite a few PRs that pnkfelix opted not to mark as triaged. In particular, a broad set of primary benchmarks improved, due to improvements to resolve (PR #118188) and a one-pass rewrite of exhaustiveness (PR #117611).

Triage done by @pnkfelix. Revision range: 4f3da903..df0295f0

Summary:

(instructions:u) mean range count
Regressions (primary) 0.6% [0.1%, 1.5%] 15
Regressions (secondary) 1.3% [0.2%, 2.4%] 16
Improvements (primary) -0.7% [-2.1%, -0.3%] 66
Improvements (secondary) -1.7% [-8.1%, -0.2%] 43
All (primary) -0.5% [-2.1%, 1.5%] 81

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

Regressions

Rollup of 4 pull requests #118319 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.4% [0.1%, 0.8%] 23
Regressions (secondary) 0.5% [0.2%, 1.0%] 11
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.4% [0.1%, 0.8%] 23
  • The bulk (in this case > 0.31%) of the primary regressions are to bitmaps and libc, in a variety of incremental modes.
  • nnethercote noted that this seems like it must be PR #118311 (“merge DefKind::Coroutine into Defkind::Closure”), and confirmed it by benchmarking that specific commit.
  • follow-up PR’s have been proposed, but we have not successfully found one that undoes the regression.
  • meanwhile, a follow-on PR, #118188, has landed that is coupled to #118311. This PR #118188 seems to have wide benefits. So it may not be worthwhile to spend time trying to figure out the regression injected by #118311.
  • not marking as triaged yet.

Improvements

Remove PredicateKind::ClosureKind #118120 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.2% [-0.3%, -0.2%] 4
Improvements (secondary) -3.8% [-8.1%, -0.5%] 14
All (primary) -0.2% [-0.3%, -0.2%] 4
  • slight improvements to clap check-{incr-full,full}, cargo check-full, and diesel doc-full

Cache flags for ty::Const #118189 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.3% [-0.3%, -0.2%] 10
Improvements (secondary) -0.3% [-0.3%, -0.2%] 3
All (primary) -0.3% [-0.3%, -0.2%] 10
  • slight improvements to bitmaps {check-full,opt-full}, serde {check-full,debug-full}, diesel check-full
  • the remaining 5 are doc-full improvements.

Indicate that multiplication in Layout::array cannot overflow #118228 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.4% [-0.5%, -0.3%] 3
Improvements (secondary) - - 0
All (primary) -0.4% [-0.5%, -0.3%] 3
  • switches to unsafe { element_size.unchecked_mul(n) } with a big ol' safety comment about why.
  • improved opt incr-patched:println for clap, image, and cargo benchmarks.

AmbiguityCause should not eagerly format strings #118267 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.4% [-0.8%, -0.2%] 5
Improvements (secondary) - - 0
All (primary) -0.4% [-0.8%, -0.2%] 5
  • improved check builds for clap {incr-full,full,incr-unchanged} and hyper {incr-full,full}

resolve: Feed the def_kind query immediately on DefId creation #118188 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.3% [-0.5%, -0.2%] 58
Improvements (secondary) -0.5% [-1.0%, -0.1%] 34
All (primary) -0.3% [-0.5%, -0.2%] 58
  • wide range of benchmarks improved on incr-unchanged and incr-patched variants: stm32f4, diesel, bitmaps, cranelift-codegen, syn, serde, et cetera.
  • as noted above with #118319, this is coupled with a PR (#118311) associated with some regressions.

Mixed

Refactor binary_search_by to use conditional moves #117722 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.4% [0.4%, 0.4%] 1
Regressions (secondary) 1.3% [1.3%, 1.4%] 2
Improvements (primary) -1.4% [-1.9%, -0.2%] 5
Improvements (secondary) -1.8% [-2.6%, -1.3%] 8
All (primary) -1.1% [-1.9%, 0.4%] 6
  • The single primary regression here seems to be a measurement blip, based on the 30-day history.
  • Even if it weren’t, the improvements would outweigh the regression.
  • Marked as triaged.

Rewrite exhaustiveness in one pass #117611 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 1.1% [1.0%, 1.1%] 2
Regressions (secondary) 1.6% [0.3%, 2.4%] 9
Improvements (primary) -0.9% [-2.0%, -0.2%] 11
Improvements (secondary) -0.2% [-0.2%, -0.2%] 1
All (primary) -0.6% [-2.0%, 1.1%] 13
  • primary improvements were to html5ever, cranelift-codegen, exa, and image.
  • unicode-normalization was the main primary regression, by up to 1.15% (check incr-full); but its worth noting that it was very close to the significance factor (1.13%) for that benchmark, so its borderline historically.
  • already marked as triaged by nnethercote

rustc: Make def_kind mandatory for all DefIds #118250 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 0.5% [0.5%, 0.5%] 2
Improvements (primary) -0.1% [-0.1%, -0.1%] 5
Improvements (secondary) -0.3% [-0.5%, -0.2%] 9
All (primary) -0.1% [-0.1%, -0.1%] 5
  • already marked as triaged by nnethercote. (regressions are confined to secondary match-stress benchmark).

Add debug_assert_nounwind and convert assert_unsafe_precondition #110303 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.4%, 0.6%] 4
Regressions (secondary) 0.2% [0.2%, 0.3%] 2
Improvements (primary) -0.4% [-0.4%, -0.4%] 1
Improvements (secondary) -0.6% [-0.6%, -0.6%] 2
All (primary) 0.3% [-0.4%, 0.6%] 5
  • already marked as triaged by nnethercote (hoped to be churn/noise).

Rollup of 7 pull requests #118405 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.4%, 0.6%] 3
Regressions (secondary) - - 0
Improvements (primary) - - 0
Improvements (secondary) -0.5% [-1.3%, -0.2%] 4
All (primary) 0.5% [0.4%, 0.6%] 3
  • regressions are confined to clap opt {full,incr-full,incr-patched:println}
  • not marking as triaged

Nominated Issues

T-compiler

  • No I-compiler-nominated issues this time.

RFC

  • “RFC: Packages as (optional) namespaces” rfcs#3243

Oldest PRs waiting for review

T-compiler

  • “Stabilize extended_varargs_abi_support” rust#116161
    • author pinged @wesley wiser
  • “Create the previous dep graph index on a background thread” rust#116375
    • cc: cjgillot
  • “Provide context when ? can’t be called because of Result<_, E>” rust#116496
    • cc: @compiler errors

Next week’s WG checkins

  • @_T-rust-analyzer by @Lukas Wirth

Next meetings' agenda draft: hackmd link