T Compiler Meeting Agenda 2023 06 08

T-compiler Meeting Agenda 2023-06-08

Announcements

Other WG meetings (calendar link)

MCPs/FCPs

  • New MCPs (take a look, see if you like them!)
  • Old MCPs (stale MCP might be closed as per MCP procedure)
    • None at this time
  • Old MCPs (not seconded, take a look)
    • “Disallow incoherent cfgs” compiler-team#610 (last review activity: about 53 days ago)
    • “2024: Decrease debuginfo generated by -Cdebuginfo=1” compiler-team#613 (last review activity: about 46 days ago)
    • “Cell Broadband Engine SPU support” compiler-team#614 (last review activity: about 53 days ago)
    • “Revise error code documentation standard” compiler-team#615 (last review activity: about 46 days ago)
    • “Runtime checks for occupied niches” compiler-team#624 (last review activity: about 11 days ago)
    • “Add support for Zephyr OS” compiler-team#629 (last review activity: about 11 days ago)
    • “Add mips64r6 option to target_arch” compiler-team#632 (last review activity: about 11 days ago)
    • “Consistently use “region” terminology in later stages of the compiler” compiler-team#634 (last review activity: about 11 days ago)
    • “Add a new --build-id flag to rustc” compiler-team#635 (last review activity: about 11 days ago)
  • Pending FCP requests (check your boxes!)
    • No pending FCP requests this time.
  • Things in FCP (make sure you’re good with it)
  • Accepted MCPs
  • Finalized FCPs (disposition merge)
    • “Make pointer_structural_match normal and warn” rust#110166
    • “rustdoc: Add search result item types after their name” rust#110688
    • “[mir-opt] SimplifyLocals should also clean up debuginfo” rust#110702
    • “Stabilize inline asm for LoongArch64” rust#111235
    • “Uplift clippy::undropped_manually_drops lint” rust#111530
    • “Uplift clippy::invalid_utf8_in_unchecked lint” rust#111543
    • “Uplift clippy::cast_ref_to_mut lint” rust#111567
    • “Uplift clippy::fn_null_check lint” rust#111717
    • “Uplift clippy::cmp_nan lint” rust#111818

WG checkins

  • @_WG-polymorphization by @davidtwco (previous checkin):

    Checkin text

  • @_WG-rls2.0 by @Lukas Wirth (previous checkin):

    We actually noticed an interesting issue. rust-analyzer now depends on the rustc_abi crate for layout calculations (required for const eval and as well as some IDE features). It is important for this crate as our dependency to be in sync with the current toolchain such that we don’t analysize things incorrectly depending on the layout that would mismatch what the current rustc thinks. The problem here is, that upstream rust-analyzer usually targets the latest stable rust for which we can be in sync manually on the master branch, but the problem arises when considering the rustup component now. Our subtree syncs target the latest rust-analyzer master branch which targets the latest released stable, and as such any beta branches (and therefore future stable releases) will sometimes be guruanteed to mismatch. We aren’t sure how to tackle this problem yet unfortunately (this is very similar to the proc-macro abi problems we had in the past intersetingly enough), related tracking issue https://github.com/rust-lang/rust-analyzer/issues/14846

Backport nominations

T-compiler stable / T-compiler beta

  • :beta: [1.71.0] “Make struct layout not depend on unsizeable tail” rust#112062
  • :beta: [1.71.0] “Fix type-inference regression in #112225” rust#112266
    • nominated by @Michael Goulet (compiler-errors)
    • Fixes #11225 (P-high), issue with type inference affecting a few crates in production
  • :stable: [1.70.0] “Fix type-inference regression in #112225” rust#112266

T-rustdoc stable / T-rustdoc beta

  • No beta nominations for T-rustdoc this time.
  • No stable nominations for T-rustdoc this time.

:back: / :shrug: / :hand:

PRs S-waiting-on-team

T-compiler

Oldest PRs waiting for review

  • “feat: split unsafe_code lint into lint group” rust#108975
    • review is assigned to Niko, re-roll?
  • “Take MIR dataflow analyses by mutable reference” rust#108293
    • cc: @eholk
  • “Fixes LTO + build-std + Oz failed to resolve undefined symbols” rust#109821 (last review activity: 2 months ago)
    • 07-06-2023 cc: @oli
  • “Validate fluent variable references in tests” rust#111269 (last review activity: about 31 days ago)
    • cc @davidtwco
  • “Implement a global value numbering MIR optimization” rust#109597 (last review activity: about 22 days ago)
    • cc: @Jak{e,ob} Degen maybe also @RalfJ (since participant in the review)

Issues of Note

Short Summary

P-critical

T-compiler

  • No P-critical issues for T-compiler at this time.

T-types

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

T-rustdoc

  • No P-critical issues for T-rustdoc at this time.

P-high regressions

P-high beta regressions

  • no new p-high regressions

Unassigned P-high nightly regressions

  • No unassigned P-high nightly regressions this time.

Performance logs

triage logs for 2023-06-06

This week was a bunch of improvements, largely driven by cutting std debuginfo in #110221 and a few MIR or codegen optimizations. No pure regressions landed this week!

Triage done by @simulacrum. Revision range: 1221e43bdf413f7c405e9b17ef19d76c88222098..adc719d7147d5e2578ce08e0b4504be44650256e

Summary:

(instructions:u) mean range count
Regressions (primary) 0.6% [0.3%, 2.0%] 11
Regressions (secondary) 0.5% [0.3%, 1.0%] 9
Improvements (primary) -1.9% [-17.4%, -0.2%] 103
Improvements (secondary) -5.2% [-17.4%, -0.2%] 116
All (primary) -1.7% [-17.4%, 2.0%] 114

0 Regressions, 1 Improvements, 12 Mixed; 2 of them in rollups 79 artifact comparisons made in total

Regressions

No regressions this week.

Improvements

Greatly decrease the size of rustc_driver.so when debuginfo is enabled #110221 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -8.8% [-17.6%, -0.5%] 17
Improvements (secondary) -7.5% [-17.2%, -0.2%] 72
All (primary) -8.8% [-17.6%, -0.5%] 17

This PR compresses and cuts some debuginfo from the shipped std library, somewhat unintentionally. This might be something we roll back if we get regression reports about it - not clear whether this changes behavior in a meaningful way for anyone.

Mixed

Rollup of 6 pull requests #112016 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.2%, 1.0%] 14
Regressions (secondary) 0.3% [0.3%, 0.5%] 6
Improvements (primary) -0.7% [-1.1%, -0.3%] 2
Improvements (secondary) -0.5% [-0.5%, -0.5%] 1
All (primary) 0.3% [-1.1%, 1.0%] 16

Roughly scattered changes, many of which are in high-variance benchmarks.

Enable MatchBranchSimplification #112001 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.3%, 0.7%] 5
Regressions (secondary) 0.6% [0.2%, 0.8%] 10
Improvements (primary) -0.3% [-0.4%, -0.2%] 4
Improvements (secondary) -0.3% [-0.3%, -0.3%] 1
All (primary) 0.2% [-0.4%, 0.7%] 9

Scattered changes, mostly a wash. Optimization targets particular code patterns which don’t occur that often in rustc most likely.

MIR: opt-in normalization of BasicBlock and Local numbering #111813 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.3% [0.3%, 0.3%] 1
Regressions (secondary) 0.7% [0.3%, 1.2%] 8
Improvements (primary) - - 0
Improvements (secondary) -1.0% [-1.5%, -0.3%] 7
All (primary) 0.3% [0.3%, 0.3%] 1

Mostly noise, probably. The new normalization is off by default.

Inline derived hash #111963 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.3% [0.3%, 0.3%] 1
Regressions (secondary) 0.5% [0.2%, 0.6%] 15
Improvements (primary) -0.3% [-0.6%, -0.2%] 16
Improvements (secondary) -0.7% [-1.4%, -0.3%] 21
All (primary) -0.2% [-0.6%, 0.3%] 17

Improvements outweigh regressions.

Use Cow in {D,Subd}iagnosticMessage. #111748 (Comparison Link)

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

Regressions limited to secondary benchmarks, and a good portion of them are bimodal benchmarks. No further investigation warranted.

Make TrustedStep require Copy #112083 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.2% [0.2%, 0.2%] 1
Regressions (secondary) 0.5% [0.2%, 0.7%] 5
Improvements (primary) -1.1% [-1.1%, -1.1%] 1
Improvements (secondary) -1.2% [-3.5%, -0.4%] 13
All (primary) -0.4% [-1.1%, 0.2%] 2

Improvements clearly outweigh regressions.

move super_relate_consts hack to normalize_param_env_or_error #111623 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.3% [0.2%, 0.6%] 8
Regressions (secondary) - - 0
Improvements (primary) -0.3% [-0.3%, -0.2%] 2
Improvements (secondary) -2.0% [-2.2%, -2.0%] 6
All (primary) 0.2% [-0.3%, 0.6%] 10

Most likely inlining or other codegen changes; roughly neutral change overall.

Enable ScalarReplacementOfAggregates in optimized builds #112002 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.4% [0.2%, 0.9%] 6
Regressions (secondary) 0.8% [0.3%, 2.3%] 15
Improvements (primary) -0.3% [-0.9%, -0.2%] 13
Improvements (secondary) -1.2% [-2.9%, -0.3%] 3
All (primary) -0.1% [-0.9%, 0.9%] 19

SROA benefits cranelift codegen and has no significant impact on runtimes, so enabling by default makes sense. It also helps with MatchBranchSimplification, enabled in a separate PR.

Enable ConstGoto and SeparateConstSwitch passes by default #112040 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 2.6% [2.6%, 2.6%] 1
Regressions (secondary) 0.2% [0.2%, 0.2%] 1
Improvements (primary) -0.5% [-1.1%, -0.2%] 72
Improvements (secondary) -0.8% [-2.0%, -0.2%] 33
All (primary) -0.4% [-1.1%, 2.6%] 73

More MIR opt enablement – some regressions here, but improvements are much more wide-ranging than the regressions.

Rollup of 7 pull requests #112253 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.4%, 0.6%] 6
Regressions (secondary) 0.9% [0.4%, 1.3%] 9
Improvements (primary) -0.4% [-0.4%, -0.4%] 1
Improvements (secondary) -0.3% [-0.4%, -0.2%] 4
All (primary) 0.4% [-0.4%, 0.6%] 7

Possibly just noise/bimodality given benchmarks and lack of significant PRs in this rollup. Not investigating further.

Only check inlining counter after recursing. #112240 (Comparison Link)

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

Changes in inlining, overall a positive despite a few regressions.

Use load+store instead of memcpy for small integer arrays #111999 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 2.2% [0.1%, 5.7%] 3
Improvements (primary) - - 0
Improvements (secondary) -0.3% [-0.3%, -0.3%] 4
All (primary) - - 0

No clear impact on rustc workloads, but may benefit others.

Nominated Issues

T-compiler

  • “-C split-debuginfo=packed generates duplicate .dwp files for shared objects” rust#109309
  • “Port PGO/LTO/BOLT optimized build pipeline to Rust” rust#112235

RFC

  • No I-compiler-nominated RFCs this time.

Next week’s WG checkins

  • @_WG-self-profile by @mw and @Wesley Wiser

Agenda draft: https://hackmd.io/pdK-8nD2TySu6dw2N7cbgw