T Compiler Meeting Agenda 2024 08 22

T-compiler Meeting Agenda 2024-08-22

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!)
    • No new proposals this time.
  • Old MCPs (stale MCP might be closed as per MCP procedure)
    • None at this time
  • Old MCPs (not seconded, take a look)
    • “Target families for executable format” compiler-team#716 (Zulip) (last review activity: 6 months ago)
    • “Partial compilation using MIR-only rlibs” compiler-team#738 (Zulip) (last review activity: 4 months ago)
    • “Add Hotpatch flag” compiler-team#745 (Zulip) (last review activity: 3 months ago)
    • “Don’t track --emit= options as part of crate SVH” compiler-team#769 (Zulip) (last review activity: about 6 days ago)
    • “Opt-in flag for absolute paths in diagnostics” compiler-team#770 (Zulip) (last review activity: about 6 days ago)
    • “--msrv=version option so the compiler can take MSRV into account when linting” compiler-team#772 (Zulip) (last review activity: about 6 days ago)
  • Pending FCP requests (check your boxes!)
    • “sanitizers: Stabilize AddressSanitizer and LeakSanitizer for the Tier 1 targets” rust#123617
    • “Add --print host-triple to print host target triple” rust#125579
    • “Don’t warn empty branches unreachable before edition 2024” rust#129103
  • Things in FCP (make sure you’re good with it)
  • Accepted MCPs
    • No new accepted proposals this time.
  • MCPs blocked on unresolved concerns
  • Finalized FCPs (disposition merge)
    • No new finished FCP (disposition merge) this time.
  • Other teams finalized FCPs
    • “Stabilize opaque type precise capturing (RFC 3617)” rust#127672
    • “Stabilize unsafe_attributes” rust#128771

WG checkins

None

Backport nominations

T-compiler beta / T-compiler stable

  • No beta nominations for T-compiler this time.
  • 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
    • First attempt to fix was in #128844 (now closed), new plan is described in issue #129023 (cc @Noah Lev (camelid))
    • PR with a feature gate option by @Boxy in #129246, @Vadim Petrochenkov prefers reverting (comment) #125915
    • Discussion in progress
  • “regression: adding a def’n for node-id NodeId(597) and def kind AnonConst but a previous def’n exists” rust#128901
    • Basically same reasoning as above

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
    • Discussed last week, seems fine to accept that breakage

Unassigned P-high nightly regressions

  • “Generated WebAssembly unexpectedly requires reference types” rust#128475
    • “this change is expected”. Documentation PR in progress in #128511 (I-compiler-nominated; see below for the nomination)

Performance logs

triage logs for 2024-08-19

A fairly noisy week (though most of that has been dropped from this report). Overall we saw several improvements, and ended the week on a net positive. Memory usage is down around 1.5-3% over the course of the week, primarily due to RawVec polymorphization and CloneToUninit impl expansion.

Triage done by @simulacrum. Revision range: 9cb1998e..4fe1e2bd

Summary:

(instructions:u) mean range count
Regressions (primary) 0.4% [0.2%, 1.7%] 124
Regressions (secondary) 0.5% [0.1%, 1.4%] 103
Improvements (primary) -1.3% [-4.3%, -0.2%] 50
Improvements (secondary) -1.4% [-3.3%, -0.2%] 15
All (primary) -0.1% [-4.3%, 1.7%] 174

1 Regressions, 1 Improvements, 3 Mixed; 1 of them in rollups 53 artifact comparisons made in total

Regressions

Fix problems with assoc expr token collection #128725 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.3% [0.2%, 0.3%] 5
Regressions (secondary) 0.3% [0.2%, 0.5%] 5
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.3% [0.2%, 0.3%] 5

Appears to be a real change in behavior (pre-merge showed fewer regressions) but this is a correctness fix, so accepting them.

Improvements

Rework MIR inlining debuginfo so function parameters show up in debuggers. #128861 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 2.3% [2.3%, 2.3%] 1
Improvements (primary) -1.2% [-1.4%, -1.1%] 6
Improvements (secondary) -1.2% [-1.3%, -1.2%] 2
All (primary) -1.2% [-1.4%, -1.1%] 6

An improvement on many current benchmarks; the one regression is a spurious change.

Mixed

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

(instructions:u) mean range count
Regressions (primary) 0.5% [0.2%, 1.5%] 89
Regressions (secondary) 0.4% [0.1%, 1.3%] 96
Improvements (primary) -1.1% [-3.8%, -0.2%] 54
Improvements (secondary) -1.2% [-2.9%, -0.3%] 13
All (primary) -0.1% [-3.8%, 1.5%] 143

An improvement on many current benchmarks, though some regressions. Overall a net positive. Also reduced memory usage by about 1% for many of our benchmarks, which is a great win!

Support reading thin archives in ArArchiveBuilder #128936 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.3% [0.2%, 0.4%] 14
Regressions (secondary) 0.4% [0.4%, 0.4%] 1
Improvements (primary) - - 0
Improvements (secondary) -2.3% [-2.3%, -2.3%] 1
All (primary) 0.3% [0.2%, 0.4%] 14

Regressions looks genuine, though relatively rare in our benchmark suite. It might be worth doing some profiling of the Rust archive writer to see if there’s opportunities for optimization since it presumably hasn’t received much attention so far.

Rollup of 6 pull requests #129202 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 0.6% [0.1%, 2.2%] 5
Improvements (primary) -0.9% [-1.7%, -0.3%] 4
Improvements (secondary) -1.4% [-2.2%, -0.6%] 2
All (primary) -0.9% [-1.7%, -0.3%] 4

Regression on match-stress looks genuine but likely doesn’t merit deeper investigation given that this is in a rollup (and I don’t see obvious match-stress-implicating PRs in the list).

Nominated Issues

T-compiler

  • “[DRAFT] #[contracts::requires(…)]” rust#128045
    • nominated by @Jieyou Xu (comment):
    • Currently our attribute infra don’t support #[namespaced::attr(<arbitrary_inner_expr>)], without falling back to raw token handling. @apiraino & jieyouxu: this should be a design meeting.
    • Looking for suitable reviewers for the attribute handling -> HIR/MIR lowering parts. jieyouxu can help look at attribute handling, but would like another reviewer to look at the HIR/MIR parts (since oli is now on leave).
  • “Document WebAssembly target feature expectations” rust#128511
    • nominated by @Jieyou Xu
    • Context: LLVM 19 update caused wasm proposals (think target features but in wasm) to become enabled-by-default (reference-types and multivalue) -> noticed by user in https://github.com/rust-lang/rust/issues/128475.
    • Expected that more wasm proposals will become enabled-by-default over time.
    • PR amends target docs for Tier 2 targets wasm32-unknown-unknown, wasm32-wasip1-threads, wasm32-wasip1, wasm32-wasip2
      • Amends baseline enabled-by-default wasm proposals
      • Documents how to produce Minimal Viable Product (MVP) wasm without these wasm proposals.
    • Does it need an FCP signoff?
    • What’s a path forward with respect to more wasm proposals (see comment) becoming enabled-by-default in future LLVM updates, causing the wasm32 target baseline expectations to be follow?
  • “type_id is not sufficiently collision-resistant” rust#129014
  • “Emit specific message for time<=0.3.35” rust#129343
    • nominated by @Jieyou Xu
    • Context: time inference regression
    • PR would add targeted detection logic for time involving the regressed method and emit a note to recommend updating time
    • Current impl does not try to probe for time version (otherwise we’d have to probe Cargo.toml etc.)
    • Do we want to merge this crate-specific diagnostic note?
    • Alternative: full revert of #99969 by @Esteban Küber in #129379 but … it’s complicated (see comment)
  • “Revert “alloc: implement FromIterator for Box”” rust#129379
    • See point above. tbh @apiraino agrees with @Josh Stone (cuviper) in comment that this is a t-libs-api decision

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