T Compiler Meeting Agenda 2022 10 06

T-compiler Meeting Agenda 2022-10-06

Announcements

Other WG meetings

MCPs/FCPs

  • New MCPs (take a look, see if you like them!)
    • “Introduce a #[rustc::*] tool attribute namespace and provide a stable #[rustc::on_unimplemented] attribute” compiler-team#560
    • “-Z instrument-xray option” compiler-team#561
  • Old MCPs (not seconded, take a look)
    • “Arbitrary annotations in compiletest” compiler-team#513 (last review activity: 5 months ago)
    • “Add #[alias] attribute to allow symbol aliasing” compiler-team#526 (last review activity: 2 months ago)
    • “Use RangeInclusive in SpanData instead of lo/hi” compiler-team#534 (last review activity: about 40 days ago)
    • “Allow informational -Z flags on stable compiler” compiler-team#542 (last review activity: about 40 days ago)
    • " Promote i586-unknown-linux-gnu to Tier 2 with Host Tools " compiler-team#543 (last review activity: about 40 days ago)
    • “Lower baseline expectations for i686 unix-like targets” compiler-team#548 (last review activity: about 26 days ago)
    • “MCP: Flag to disable extended error info.” compiler-team#550 (last review activity: about 7 days ago)
    • “New Tier-3 target proposal: powerpc64-ibm-aix” compiler-team#553 (last review activity: about 26 days ago)
    • “configurable rustc timeout for compiletest tests” compiler-team#554 (last review activity: about 26 days ago)
    • “MCP: Raise UEFI Targets to Tier-2” compiler-team#555 (last review activity: about 26 days ago)
    • “Raise minimum supported macOS and iOS versions” compiler-team#556 (last review activity: about 26 days ago)
    • “Rustc Contributor Program Major Change Proposal” compiler-team#557 (last review activity: about 13 days ago)
  • Pending FCP requests (check your boxes!)
    • “Only apply ProceduralMasquerade hack to older versions of rental” rust#94063
    • “Remove save-analysis.” rust#101841
  • Things in FCP (make sure you’re good with it)
    • “Elaborate supertrait bounds when triggering unused_must_use on impl Trait” rust#102287
  • Accepted MCPs
  • Finalized FCPs (disposition merge)
    • “Tracking Issue for asm_sym” rust#93333
    • “Allow transmutes between the same types after erasing lifetimes” rust#101520

WG checkins

  • @_WG-mir-opt MIR Optimizations by @oli (previous checkin):

    Checkin text

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

    No updates, wg members are busy elsewhere.

Backport nominations

T-compiler stable / T-compiler beta

  • :beta: “Revert perf-regression 101620” rust#102064
    • Reverts #101862 and #101620
    • nominated by @cjgillot (comment) to backport the perf. regression fix
    • some benchmarks still show a minus sign (triaged and acceptable, see comment)
  • :beta: “Fix associated type bindings with anon const in GAT position” rust#102336
    • fixes #102333, a P-medium ICEs with GATs (on nightly+beta)
    • nominated by Wesley to let it go with GATs stabilization
  • No stable nominations for T-compiler this time.

T-rustdoc beta / T-rustdoc stable

  • No backport nominations for T-rustdoc this time.

:back: / :shrug: / :hand:

PRs S-waiting-on-team

T-compiler

  • “Only apply ProceduralMasquerade hack to older versions of rental” rust#94063
    • PR is now waiting for T-compiler to approve (or raise concerns)
  • 2 other hidden issue waiting on teams or progressing

Oldest PRs waiting for review

T-compiler

  • “Allow impl Fn() -> impl Trait in return position” rust#93582 (last review activity: about 41 days ago)
    • ping @nikomatsakis for another round of review
  • “Fix off-by-one error in X87DoubleExtended::from_bits” rust#100685 (last review activity: about 40 days ago)
    • @eddyb changes review assignment to @Wesley Wiser for licensing about the tests in this PR (comment)
  • “Support #[global_allocator] without the allocator shim” rust#86844 (last review activity: 2 months ago)
    • @pnkfelix ready for another round of review
  • “Track -Cprofile-use and -Cprofile-sample-use value by file hash, not file path” rust#100413 (last review activity: about 38 days ago)
  • “Add kernel-address sanitizer support for freestanding targets” rust#99679 (last review activity: 2 months ago)
    • @cuviper ready for another round of review

Issues of Note

Short Summary

P-critical

T-compiler

  • “Performance regression in 1.64+ when BufReader inner reader doesn’t fill the buffer” rust#102727
    • regression shows only on debug runs (primarily from evaluating the predicates of intrinsics::assume)
    • issue priority not completely clear: is this a P-critical?

T-rustdoc

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

P-high regressions

P-high beta regressions

  • “codegen regression for bool” rust#101048
    • @Nikita Popov self-assigned
  • “ICE: Broken MIR in Item(…) with let_else usage (use of local which has no storage here)” rust#102317
    • assigned P-high out of caution and for visibility (priority can be lowered if it’s the case)
    • @__Ding Xiang Fei self-assigned for investigation

Unassigned P-high nightly regressions

  • No unassigned P-high nightly regressions this time.

Performance logs

triage logs for 2022-10-04

A great week, with 170 primary benchmark scenarios seeing improvement. Every PR flagged by perf provided at least some wins, and perhaps more impressive: No rollup PR’s were flagged by perf this week! Furthermore, cjgillot fixed an issue where incremental compilation was being unnecessarily hindered by our span and lint system. Great work everyone!

Triage done by @pnkfelix. Revision range: d9297d22..02cd79af

Summary:

(instructions:u) mean range count
Regressions (primary) 1.2% [0.4%, 1.9%] 2
Regressions (secondary) 0.5% [0.4%, 0.6%] 6
Improvements (primary) -2.4% [-31.0%, -0.2%] 170
Improvements (secondary) -1.5% [-5.8%, -0.3%] 159
All (primary) -2.4% [-31.0%, 1.9%] 172

0 Regressions, 2 Improvements, 7 Mixed; 0 of them in rollups 30 Untriaged Pull Requests 54 artifact comparisons made in total

Regressions

None ‽

Improvements

More lexer improvements #102302 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 2.2% [2.2%, 2.2%] 1
Improvements (primary) -0.5% [-1.0%, -0.2%] 80
Improvements (secondary) -0.8% [-1.8%, -0.3%] 75
All (primary) -0.5% [-1.0%, -0.2%] 80

Inline a few functions. #102387 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.4% [-1.0%, -0.2%] 60
Improvements (secondary) -0.8% [-2.3%, -0.3%] 42
All (primary) -0.4% [-1.0%, -0.2%] 60

Mixed

Rewrite and refactor format_args!() builtin macro. #100996 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 1.5% [1.1%, 2.0%] 6
Improvements (primary) -0.3% [-0.7%, -0.2%] 7
Improvements (secondary) -1.2% [-1.5%, -0.6%] 7
All (primary) -0.3% [-0.7%, -0.2%] 7

rustdoc: Remove clean::TraitWithExtraInfo and queryify is_notable_trait #102384 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.3% [0.3%, 0.3%] 1
Regressions (secondary) 2.7% [2.7%, 2.7%] 1
Improvements (primary) - - 0
Improvements (secondary) -1.4% [-1.7%, -1.2%] 12
All (primary) 0.3% [0.3%, 0.3%] 1

Fix perf regression from TypeVisitor changes #101893 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 1.4% [1.2%, 1.6%] 6
Improvements (primary) - - 0
Improvements (secondary) -1.3% [-2.9%, -0.5%] 9
All (primary) - - 0

Shrink hir::def::Res #101887 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 1.2% [0.6%, 1.6%] 7
Improvements (primary) -0.2% [-0.3%, -0.2%] 5
Improvements (secondary) -0.3% [-0.4%, -0.3%] 2
All (primary) -0.2% [-0.3%, -0.2%] 5

Move lint level source explanation to the bottom #101986 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 1.0% [0.9%, 1.0%] 2
Improvements (primary) -0.3% [-0.3%, -0.2%] 3
Improvements (secondary) -0.9% [-1.4%, -0.3%] 8
All (primary) -0.3% [-0.3%, -0.2%] 3

Compute lint levels by definition #102236 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.6% [0.3%, 0.9%] 14
Regressions (secondary) 1.0% [0.2%, 2.0%] 20
Improvements (primary) -3.2% [-30.2%, -0.2%] 84
Improvements (secondary) -1.1% [-2.8%, -0.3%] 52
All (primary) -2.7% [-30.2%, 0.9%] 98
  • based on reading of #101620, this was an important step for making incremental compilation apply to a much broader set of input code.
  • the performance gains reported by perf seem well worth the cost that it registered.
  • marked as triaged.

Mark Cell::replace() as #[inline] #102548 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.9% [0.4%, 1.8%] 3
Regressions (secondary) 0.9% [0.2%, 1.3%] 10
Improvements (primary) -0.3% [-1.2%, -0.2%] 14
Improvements (secondary) -0.4% [-1.9%, -0.2%] 12
All (primary) -0.1% [-1.2%, 1.8%] 17
  • PR had intermediate perf run reported stats that are somewhat better than what we actually ended up with in the end.
  • did not mark as triaged; pinged a reviewer to confirm that this still meets acceptable criteria from their POV.

Nominated Issues

T-compiler

  • “Example iterator fold vs. loop emit different code” rust#99656
    • perf. regression affecting WASM and x86_64 targets
    • shows intermittently and fluctuates since quite old releases (1.33 ~ 1.56)
    • @_mqudsi suggests it could to be caused by multiple issues (comment)
    • nominated by @apiraino: does it need to be prioritized i.e. does it look bad enough to require a closer look?
  • “Constify TypeId ordering impls” rust#101698
    • @raldone01 adds some background (comment)
    • @scottmcm: T-lang sees no blockers for nightly but stabilization should be well thought first (comment)
    • @Mara nominated for T-compiler: “will this get in the way of any future plans there might be with future typeid implementations? (And if so, is that a problem?)” (comment)
  • “extern “C” functions don’t generate the same IR definitions as clang on x86, causing problems with cross-language LTO” rust#102174
    • an LLVM upgrade led to inlining code that breaks stuff (affects Firefox builds)
    • @Nikita Popov mentions that the issue is limited to the cross-language LTO case only" (comment)
    • @eddyb adds more insights (comment)
    • nominated by @apiraino: I need context to understand the priority of this regression, what the actionable could be (and perhaps find an owner?)

RFC

  • No nominated RFCs for T-compiler this time.

Next week’s WG checkins

  • @_WG-rls2.0 by @Lukas Wirth
  • @_WG-self-profile by @mw and @Wesley Wiser