owned this note   changed 6 months ago  
Published Linked with GitHub

T-compiler Meeting Agenda 2024-09-19

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!)
  • Old MCPs (not seconded, take a look)
  • 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
    • "make unsupported_calling_conventions a hard error" rust#129935
  • Things in FCP (make sure you're good with it)
  • Accepted MCPs
  • MCPs blocked on unresolved concerns
  • Finalized FCPs (disposition merge)
    • "Check WF of source type's signature on fn pointer cast" rust#129021
    • "Relate receiver invariantly in method probe for Mode::Path" rust#129073
    • "(Anti-)regression between Rust 1.78.0 and Rust 1.79.0 with struct containing Cow<[Self]>" rust#129541
  • Other teams finalized FCPs
    • "Proposal: stabilize const_refs_to_static" rust#128183
    • "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
    • "(Anti-)regression between Rust 1.78.0 and Rust 1.79.0 with struct containing Cow<[Self]>" rust#129541
    • "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: "Update LLVM to 19 327ca6c" rust#130212
    • Authored by DianQK
    • will fix #129887, a P-high crash on compiling on aarch64
  • :beta: "Revert #129749 to fix segfault in LLVM" rust#130477
    • Authored by tmandry
    • reverts some LLVM breaking changes (see comment). Also fixes #130333
  • :beta: "Check params for unsafety in THIR" rust#130531
    • Authored by compiler-errors
    • Fixes p-critical unsoundness #130528
  • 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

  • "Unsoundness: Patterns in function parameters are not checked for union access" rust#130528
    • fixed by #130531

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
    • likely fixed by #129073. In any case on the radar of @_lcnr

Unassigned P-high nightly regressions

  • No unassigned P-high nightly regressions this time.

Performance logs

triage logs for 2024-09-16

A relatively quiet week, with overall neutral performance across our set of key
metrics (instructions, cycles, memory).

Triage done by @simulacrum.
Revision range: 263a3aee..170d6cb8

Summary:

(instructions:u) mean range count
Regressions ❌
(primary)		 0.7% [0.2%, 4.0%] 52
Regressions ❌
(secondary)		 0.7% [0.2%, 1.4%] 48
Improvements ✅
(primary)		 -0.8% [-2.6%, -0.2%] 46
Improvements ✅
(secondary)		 -0.5% [-1.1%, -0.1%] 25
All ❌✅ (primary) 0.0% [-2.6%, 4.0%] 98

2 Regressions, 3 Improvements, 4 Mixed; 2 of them in rollups
54 artifact comparisons made in total

Regressions

interpret: make typed copies lossy wrt provenance and padding #129778 (Comparison Link)

(instructions:u) mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 2.1% [0.2%, 4.7%] 13
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

Most of the performance is recovered in https://github.com/rust-lang/rust/pull/130197.

generalize: track relevant info in cache key #130194 (Comparison Link)

(instructions:u) mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 1.0% [0.4%, 1.3%] 16
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -0.2% [-0.2%, -0.2%] 1
All ❌✅ (primary) - - 0

Soundness fix and regressions only in secondary benchmarks.

Improvements

rustdoc: use strategic boxing to shrink clean::Item #129789 (Comparison Link)

(instructions:u) mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -0.5% [-1.0%, -0.2%] 15
Improvements ✅
(secondary)		 -0.5% [-0.6%, -0.3%] 19
All ❌✅ (primary) -0.5% [-1.0%, -0.2%] 15

rustdoc: unify the short-circuit on all lints #129975 (Comparison Link)

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

Rollup of 10 pull requests #130253 (Comparison Link)

(instructions:u) mean range count
Regressions ❌
(primary)		 0.2% [0.2%, 0.2%] 1
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -0.8% [-1.5%, -0.2%] 13
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) -0.7% [-1.5%, 0.2%] 14

Mixed

Revert "Stabilize -Znext-solver=coherence" #130249 (Comparison Link)

(instructions:u) mean range count
Regressions ❌
(primary)		 3.7% [2.9%, 4.5%] 7
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 -0.5% [-0.9%, -0.2%] 13
Improvements ✅
(secondary)		 -0.5% [-0.7%, -0.1%] 9
All ❌✅ (primary) 1.0% [-0.9%, 4.5%] 20

Largely resets to the previous state on a couple benchmarks. Expected given the revert.

Rollup of 5 pull requests #130281 (Comparison Link)

(instructions:u) mean range count
Regressions ❌
(primary)		 0.6% [0.6%, 0.6%] 1
Regressions ❌
(secondary)		 0.2% [0.2%, 0.2%] 1
Improvements ✅
(primary)		 -0.2% [-0.2%, -0.2%] 1
Improvements ✅
(secondary)		 -1.8% [-2.0%, -1.6%] 6
All ❌✅ (primary) 0.2% [-0.2%, 0.6%] 2

Regressions are minor enough that given the rollup and lack of obviously
relevant PRs I'm just marking as triaged. The primary regressed benchmark is
webrender and only in full LLVM builds which doesn't seem like it maps well
onto the contained PRs.

Rescope temp lifetime in if-let into IfElse with migration lint #107251 (Comparison Link)

(instructions:u) mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 0.6% [0.2%, 1.3%] 19
Improvements ✅
(primary)		 -0.2% [-0.2%, -0.2%] 1
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) -0.2% [-0.2%, -0.2%] 1

Seems like the regression is primarily looking like a revert of #127313…
@cjgillot do you have thoughts on what the delta here might be? I don't see a
very obvious cause here. Regressions are minor enough that I'm not worried, but
just checking if there's an easy explanation.

Simplify the canonical clone method and the copy-like forms to copy #128299 (Comparison Link)

(instructions:u) mean range count
Regressions ❌
(primary)		 0.4% [0.2%, 0.8%] 6
Regressions ❌
(secondary)		 0.4% [0.2%, 2.1%] 16
Improvements ✅
(primary)		 -0.8% [-1.7%, -0.3%] 12
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) -0.4% [-1.7%, 0.8%] 18

Likely expected regressions from direct changes to codegen, possibly giving
LLVM more freedom to optimize:

This pass improves performance by reducing the amount of MIR. The generated
copies will get merged/expanded/whatever by LLVM, with a completely different
heuristic.

(https://github.com/rust-lang/rust/pull/128299#pullrequestreview-2217057820)

Nominated Issues

T-compiler

  • "Linking error with aws_lc_rs" rustc_codegen_cranelift#1520
    • An AWS cryptographic crate fails linking using cranelift, relevant code snippet from the crate (link): 
      ​​​​pub type AES_KEY = aes_key_st;
​​​​extern "C" {
​​​​    #[link_name = "\u{1}aws_lc_0_20_0_AES_set_encrypt_key"]
​​​​    pub fn AES_set_encrypt_key(
​​​​        key: *const u8,
​​​​        bits: ::std::os::raw::c_uint,
​​​​        aeskey: *mut AES_KEY,
​​​​    ) -> ::std::os::raw::c_int;
​​​​}
    • From the Zulip thread:

      C compilers are expected to mangle symbol names on several platforms. For example for Mach-O exported functions are expected to start with an _, while on Windows a suffix like @4 may get added in some cases. To disable this mangling LLVM supports prefixing the symbol name with \x01. No other backend implements this right now afaik. As it turns out rust-bindgen started unconditionally using this 7 years ago.

    • @bjorn3 asks: do we want to guarantee that it will remain working and implement it in all existing codegen backends, or is this considered something that shouldn't be used on stable code at all and should rust-bindgen stop using it when not strictly necessary, like is the case most of the time?
    • This is where bindgen does this (link)

RFC

  • No I-compiler-nominated RFCs this time.

Oldest PRs waiting for review

T-compiler

  • TODO

Next week's WG checkins

None

Next meetings' agenda draft: hackmd link

Select a repo