T Compiler Meeting Agenda 2023 09 14

T-compiler Meeting Agenda 2023-09-14

Announcements

Other WG meetings (calendar link)

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)
    • “Consistently use "region" terminology in later stages of the compiler” compiler_team#634 (Zulip) (last review activity: 3 months ago)
    • “Add a new --build-id flag to rustc” compiler_team#635 (Zulip)) (last review activity: 3 months ago)
    • “Simplify and improve explicitness of the check-cfg syntax” compiler_team#636 (Zulip) (last review activity: 3 months ago)
    • “Add support for visionOS targets” compiler_team#642 (Zulip) (last review activity: about 12 days ago)
    • “Add illumos Tier3 targets” compiler_team#644 (Zulip) (last review activity: about 51 days ago)
    • “Match the behavior of strip and split-debuginfo across platforms” compiler_team#669 (Zulip) (last review activity: about 9 days ago)
    • “Allow overriding default codegen backend on a per-target basis” compiler_team#670 (Zulip) (last review activity: about 6 days ago)
    • “Add infrastructure to "compute the ABI of a Rust type, described as a C type"” compiler_team#672 (Zulip) (last review activity: about 6 days ago)
  • Pending FCP requests (check your boxes!)
    • “Retire the mailing list and make all decisions on zulip” compiler_team#649 (Zulip)
    • " Add type field to distinguish json diagnostic outputs" compiler_team#673 (Zulip)
    • “Tracking issue for dyn upcasting coercion” rust#65991
    • “Add allow-by-default lint for unit bindings” rust#112380
    • “stabilize combining +bundle and +whole-archive link modifiers” rust#113301
    • “Support overriding warnings level for a specific lint via command line” rust#113307
  • Things in FCP (make sure you’re good with it)
  • Accepted MCPs
    • “Make unknown/renamed/removed lints passed via command line respect lint levels” compiler-team#667
  • Finalized FCPs (disposition merge)
    • None

WG checkins

  • T-types checkin by @Jack Huey (HackMD link)
    • General team things
      • We have an in-person meetup in Brussels next month prior to EuroRust
        • Let us know if there’s anything you’d like us to discuss
    • Planning to open a stabilization report “any day now” for AFIT/RPITIT
    • #114740, #115008, #114933 all open to try to fix unsoundness w.r.t TAITs
    • Ongoing work to generate a-mir-formality code via SMIR
    • Work on new trait solver: overflow handling and provisional cache
    • There’s a WIP stabilization report for trait upcasting
    • Open FCP for negative impls
    • @lqd has prepared a PR that reimplements the existing borrow-check in a polonius style

Backport nominations

T-compiler stable / T-compiler beta

  • :beta: [1.73] “MCP661: Move wasm32-wasi-preview1-threads target to Tier 2” rust#115345
    • (previously Tier 3)
    • nominated by @Wesley Wiser (comment): PR author recommends backport as this PR bumps the version of wasi-libc we use which fixes some issues with the target. The target is now Tier 2 in 1.73 so backporting the fix will improve the state of the target in its first Tier 2 release.
  • :beta: [1.73] “fix(resolve): update def if binding is warning ambiguity” rust#115389
    • Fixes beta regression #115774 from the latest crater run, hitting a few crates (so potentially annoying?)
  • :beta: [1.73] “Don’t require Drop for [PhantomData<T>; N] where N and T are generic, if T requires Drop” rust#115527
    • authored by @oli
    • Closes fixes #115403 and #115410 (also raised an error in the latest crater run, see comment)
  • :beta: [1.73] “Only suggest turbofish in patterns if we may recover” rust#115785
    • authored by @fmease
    • closes #115780 a perhaps P-high regression (produces an error falsely suggesting to use the :: operator)
  • :beta: [1.73] “Expose try_destructure_mir_constant_for_diagnostics directly to clippy” rust#115819
    • fresh from @oli
    • hopefully squashing #83085: avoid clippy trying to use the query in ways that incremental caching will inevitably cause problems with
  • No stable nominations for T-compiler this time.

PRs S-waiting-on-team

T-compiler

Issues of Note

Short Summary

P-critical

T-compiler

T-types

  • “RPIT hidden types can be ill-formed” rust#114728
    • will be fixed by rust#114933, authored by @aliemjay, being reviewed

T-rustdoc

  • “ICE when building documentation: DefId(20:797 …) does not have a “object_lifetime_default”” rust#115179

P-high regressions

P-high beta regressions

  • “regression: as_place unwrap None” rust#115778
    • @apiraino assigned P-high (but unsure about priority), needs bisection and maybe a bit more context. Might be related to #110453
  • “regression: generic args in patterns require the turbofish syntax” rust#115780
    • fixed by #115785

Unassigned P-high nightly regressions

  • No unassigned P-high nightly regressions this time.

Performance logs

triage logs for 2023-09-13

An interesting week. We saw a massive improvement to instruction-counts across over a hundred benchmarks, thanks to #110050 an improved encoding scheme for the dependency graphs that underlie incremental-compilation. However, these instruction-count improvements did not translate to direct cycle time improvements. We also saw an improvement to our artifact sizes due to #115306. Beyond that, we had a scattering of small regressions to instruction-counts that were justified because they were associated with bug fixes.

Triage done by @pnkfelix. Revision range: 15e52b05..7e0261e7

Summary:

(instructions:u)meanrangecount
Regressions (primary)2.8%[0.7%, 10.2%]11
Regressions (secondary)1.5%[0.4%, 7.7%]9
Improvements (primary)-1.7%[-5.9%, -0.2%]112
Improvements (secondary)-1.3%[-2.7%, -0.4%]41
All (primary)-1.3%[-5.9%, 10.2%]123

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

Regressions

Add FreezeLock type and use it to store Definitions #115401 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.3%[0.2%, 0.4%]11
Regressions (secondary)0.3%[0.3%, 0.3%]1
Improvements (primary)--0
Improvements (secondary)--0
All (primary)0.3%[0.2%, 0.4%]11
  • The impact here is hypothesized to be due to serial/parallel trade-off; we benchmark the serial case and observe a small regression, while the parallel case is observing an improvement of roughly the same caliber.
  • Marked as triaged

Rollup of 6 pull requests #115672 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)4.2%[0.8%, 9.8%]5
Regressions (secondary)--0
Improvements (primary)--0
Improvements (secondary)--0
All (primary)4.2%[0.8%, 9.8%]5

Use the same DISubprogram for each instance of the same inlined function within a caller #115417 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)1.0%[0.6%, 1.3%]3
Regressions (secondary)--0
Improvements (primary)--0
Improvements (secondary)--0
All (primary)1.0%[0.6%, 1.3%]3
  • already marked as triaged
  • regression was expected, though we may be able to claw back performance after resolving rust#115455

Improvements

Span tweaks #115594 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)--0
Improvements (primary)-0.4%[-0.4%, -0.4%]1
Improvements (secondary)-0.4%[-0.5%, -0.3%]6
All (primary)-0.4%[-0.4%, -0.4%]1

Disentangle Debug and Display for Ty. #115661 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)--0
Improvements (primary)-0.3%[-0.3%, -0.2%]4
Improvements (secondary)-0.3%[-0.5%, -0.2%]3
All (primary)-0.3%[-0.3%, -0.2%]4

Mixed

Represent MIR composite debuginfo as projections instead of aggregates #115252 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)3.9%[3.9%, 3.9%]1
Regressions (secondary)--0
Improvements (primary)-0.3%[-0.3%, -0.3%]2
Improvements (secondary)-0.4%[-0.4%, -0.3%]4
All (primary)1.1%[-0.3%, 3.9%]3
  • The single regression is to exa-0.10.1-opt-full
  • However, nnethercote noted that this PR introduced broad (if small) regressions to linked artifact (aka binary) sizes (in both opt and debug settings)
  • not marking as triaged

Use a specialized varint + bitpacking scheme for DepGraph encoding #110050 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)--0
Regressions (secondary)0.5%[0.3%, 0.8%]4
Improvements (primary)-1.7%[-5.8%, -0.3%]104
Improvements (secondary)-1.4%[-2.9%, -0.5%]32
All (primary)-1.7%[-5.8%, -0.3%]104
  • on its surface, the improvements to instruction counts here clearly outweigh the regressions
  • it is worth noting that the cycle counts did not see the same trends; there were zero improvements and 7 primary regressions to cycle counts.
  • still, marking as triaged; this PR has gone through enough performance evaluation already.

Rollup of 7 pull requests #115665 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.7%[0.6%, 0.7%]2
Regressions (secondary)0.6%[0.5%, 0.7%]5
Improvements (primary)--0
Improvements (secondary)-0.3%[-0.3%, -0.3%]1
All (primary)0.7%[0.6%, 0.7%]2
  • primary regressions were helloworld-check (incr-unchanged and incr-patched:println)
  • marking as triaged; not worth investigating a rollup for that benchmark.

Avoid a source_span query when encoding Spans into query results #115657 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.4%[0.3%, 0.4%]2
Regressions (secondary)0.7%[0.4%, 1.0%]7
Improvements (primary)-0.4%[-0.4%, -0.4%]2
Improvements (secondary)-0.5%[-0.6%, -0.4%]4
All (primary)-0.0%[-0.4%, 0.4%]4
  • primary regressions are to diesel-check (full and incr-full).
  • This is fixing a soundness issue with the dep-graph maintenance; therefore, these regressions seem tolerable.
  • Marking as triaged

Encode only MIR reachable from other crates #115306 (Comparison Link)

(instructions:u)meanrangecount
Regressions (primary)0.8%[0.3%, 2.4%]15
Regressions (secondary)1.9%[0.3%, 9.1%]7
Improvements (primary)-1.3%[-2.7%, -0.4%]12
Improvements (secondary)-0.9%[-1.2%, -0.7%]5
All (primary)-0.1%[-2.7%, 2.4%]27
  • the big (>1%) primary regressions were to three check-incr-unchanged cases: cranelift-codegen-0.82.1, html5ever-0.26.0, and hyper-0.14.18
  • the regressions seem unfortunate, but tolerable given the improvement to linked artifact sizes
  • marking as triaged

Nominated Issues

T-compiler

  • None

RFC

  • No I-compiler-nominated RFCs this time.

Oldest PRs waiting for review

T-compiler

  • “Fix ICE when trying to convert ConstKind::Error to usize” rust#113712(last review activity: about 46 days ago)
    • cc: @uwu
  • “Pretty-print argument-position impl trait to name it.” rust#113955(last review activity: about 31 days ago)
    • cc @Waffle Lapkin

Next week’s WG checkins

  • @_WG-mir-opt by @oli
  • @_T-rust-analyzer by @Lukas Wirth

Next meetings’ agenda draft: https://hackmd.io/CG_A-TBVTBqsH_mfID3Fvg