T Compiler Meeting Agenda 2022 08 18

T-compiler Meeting Agenda 2022-08-18

Announcements

Other WG meetings

MCPs/FCPs

  • New MCPs (take a look, see if you like them!)
  • Old MCPs (not seconded, take a look)
    • “Arbitrary annotations in compiletest” compiler-team#513 (last review activity: 3 months ago)
    • “Add support for the LoongArch architecture” compiler-team#518 (last review activity: 2 months ago)
    • “Add #[alias] attribute to allow symbol aliasing” compiler-team#526 (last review activity: about 27 days ago)
  • Pending FCP requests (check your boxes!)
    • “Stabilize raw-dylib for non-x86” rust#99916
  • Things in FCP (make sure you’re good with it)
    • No FCP requests this time.
  • Accepted MCPs
  • Finalized FCPs (disposition merge)
    • “Stabilize -Zgcc-ld=lld as -Clink-self-contained=linker -Clinker-flavor=gcc-lld” compiler-team#510
    • “Tracking Issue for “unsafe blocks in unsafe fn” (RFC #2585)” rust#71668
    • “Increase the minimum linux-gnu versions” rust#95026
    • “Remove migrate borrowck mode” rust#95565
    • “Modify MIR building to drop repeat expressions with length zero” rust#95953
    • “Lang: Stabilize usage of rustc_nonnull_optimization_guaranteed on -1” rust#97122
    • “Remove a back-compat hack on lazy TAIT” rust#97346
    • “Make outlives::{components,verify} agree” rust#97406
    • “[RFC] Support .comment section like GCC/Clang (!llvm.ident)” rust#97550
    • “make cenum_impl_drop_cast deny-by-default” rust#97652
    • “make const_err show up in future breakage reports” rust#97743
    • “lub: don’t bail out due to empty binders” rust#97867
    • “allow unions with mutable references and tuples of allowed types” rust#97995
    • “do not mark interior mutable shared refs as dereferenceable” rust#98017
    • “session: stabilize split debuginfo on linux” rust#98051
    • “relate closure_substs.parent_substs() to parent fn in NLL” rust#98835

WG checkins

Backport nominations

T-compiler beta / T-compiler stable

  • :beta: “Revert “Rollup merge of #97346 - JohnTitor:remove-back-compat-hacks, …” rust#99860
    • opened by @oli, fixes rust#99536, P-high a stable regression
    • r’ed by @lcnr
    • needs a Felix sign-off?
  • :beta: “Fix #96847” rust#100307
    • opened and beta nominated by @nnethercote comment
    • fixes rust#96847 (an ICE found by a fuzzer)
  • :beta: “Revert let_chains stabilization” rust#100538
    • opened by @Nilstrieb (Zulip topic)
    • @Wesley Wiser approved and suggests T-compiler takes a look (comment)
    • Note: T-rustdoc was just autotagged by rustbot
  • :beta: “[BETA] Use node_type_opt to skip over generics that were not expected” rust#100548
    • PR authored by @Michael Goulet (compiler-errors), closes a crater run regression rust#100541
  • :beta: “[BETA] Delay formatting trimmed path until lint/error is emitted” rust#100549
    • PR authored by @Michael Goulet (compiler-errors), closes a crater run regression rust#100542
  • No stable nominations for T-compiler this time.

T-rustdoc beta / T-rustdoc stable

  • 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

  • “Tracking issue for dyn upcasting coercion” rust#65991
    • discussed in previous meeting (Felix left comment on github)
  • “Revert “Revert “Allow dynamic linking for iOS/tvOS targets.””” rust#100636
    • Issue is T-compiler nominated
  • (2 issues hidden since they’re WIP or waiting on other teams)

Oldest PRs waiting for review

T-compiler

  • “Refactor metadata emission to avoid visiting HIR” rust#98867 (last review activity: about 45 days ago)
    • @cjgillot author and self-assigned reviewer: ping?
  • “Remove EntryKind from metadata.” rust#98960 (last review activity: about 43 days ago)
    • cc @Esteban Küber autoassigned for review
  • “Replace Body::basic_blocks*() with field access” rust#99027 (last review activity: about 41 days ago)
    • cc @oli
  • “Rework definition of MIR phases to more closely reflect semantic concerns” rust#99102 (last review activity: about 39 days ago)
    • cc @oli

Issues of Note

Short Summary

P-critical

T-compiler

  • “Wrong cast of u16 to usize on aarch64” rust#97463
    • Will be fixed by rust#97800 (pr has some tests failing)
  • “Regression in consteval: error[E0080]: could not evaluate static initializer (unable to turn pointer into raw bytes)” rust#99923
    • discussed last week in the context of PR rust#100229
    • decided to leave #99923 both open and P-critical
  • “ICE when compiling nalgebra 0.25.4 in Release mode” rust#100550
    • that release from 2021 doesn’t compile anymore in beta. newer version up to current master do (see Wg-prio comments)
    • @cjgillot commented (see comment), self assigned and provided a patch rust#100571 to at least remove the ICE (review is in progress)

T-rustdoc

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

P-high regressions

P-high beta regressions

  • “nightly 2022-06-29 rejects previously compiling code with missing trait implementations” rust#99536
    • handled by a revert PR rust#99860 authored by @_oli
  • “Anon lifetime in impl Trait no longer suggests adding a lifetime parameter” rust#100615
    • flagged as P-high because of the confusing diag. regression, open to reassess priority
    • @_TaKO8Ki self-assigned

Unassigned P-high nightly regressions

  • No unassigned P-high nightly regressions this time.

Performance logs

triage logs for 2022-08-16

A fairly quiet week for performance, with the exception of the LLVM 15 upgrade which resulted in many changes, mostly to the positive.

Triage done by @simulacrum. Revision range: cc4dd6fc9f1a5c798df269933c7e442b79661a86..14a459bf37bc19476d43e0045d078121c12d3fef

Summary:

(instructions:u) mean max count
Regressions (primary) 0.7% 7.7% 62
Regressions (secondary) 1.3% 5.0% 51
Improvements (primary) -1.8% -6.9% 93
Improvements (secondary) -2.4% -22.0% 128
All (primary) -0.8% 7.7% 155

2 Regressions, 4 Improvements, 2 Mixed; 1 of them in rollups 38 artifact comparisons made in total

Regressions

consider unnormalized types for implied bounds #99217 (Comparison Link)

(instructions:u) mean max count
Regressions (primary) 0.7% 1.9% 37
Regressions (secondary) 2.0% 2.4% 11
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.7% 1.9% 37

This regression is caused by a soundness fix. #99725 recovered a small amount of the lost performance here.

rustdoc: Merge source code pages HTML elements together #100429 (Comparison Link)

(instructions:u) mean max count
Regressions (primary) 0.6% 1.4% 12
Regressions (secondary) 1.8% 3.1% 8
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.6% 1.4% 12

The regressions are limited to doc benchmarks and the underlying change reduces the number of DOM elements generated by ~30% across a few samples (see PR description for details). Those sizes aren’t measured by perf.rust-lang.org today, so there’s no reflection of that here, but the size improvement is a welcome one.

Improvements

Reoptimize layout array #99174 (Comparison Link)

(instructions:u) mean max count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.4% -0.8% 10
Improvements (secondary) -0.8% -1.2% 5
All (primary) -0.4% -0.8% 10

Remove manual implementations of HashStable for hir::Expr and hir::Ty. #100237 (Comparison Link)

(instructions:u) mean max count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.2% -0.2% 3
Improvements (secondary) - - 0
All (primary) -0.2% -0.2% 3

Possibly spurious, but leaving in the report – bitmaps-3.1.0 check typically does not see this much impact on a run to run basis so it’s probably at least partially real.

Shrink ast::Attribute. #100441 (Comparison Link)

(instructions:u) mean max count
Regressions (primary) 0.6% 0.6% 1
Regressions (secondary) 0.2% 0.2% 1
Improvements (primary) -0.6% -1.4% 22
Improvements (secondary) -1.0% -1.5% 23
All (primary) -0.5% -1.4% 23

Mostly an improvement in doc benchmarks, though a few check benchmarks improved as well. An excellent win for memory usage, with up to 5.8% wins on helloworld.

passes: load defined_lib_features query less #100328 (Comparison Link)

(instructions:u) mean max count
Regressions (primary) - - 0
Regressions (secondary) 0.8% 1.5% 3
Improvements (primary) -0.4% -0.4% 1
Improvements (secondary) -1.2% -1.8% 8
All (primary) -0.4% -0.4% 1

The regressions here appear to be spurious, with performance recovering on the next commit. Otherwise, this is a good improvement.

Mixed

Rollup of 13 pull requests #100426 (Comparison Link)

(instructions:u) mean max count
Regressions (primary) 0.3% 0.4% 4
Regressions (secondary) 0.5% 0.7% 5
Improvements (primary) -0.9% -0.9% 1
Improvements (secondary) -1.8% -1.8% 1
All (primary) 0.0% -0.9% 5

Small-ish regression narrowed down to #99337, some active work in this area of rustdoc is likely to help here soon.

Update to LLVM 15 #99464 (Comparison Link)

(instructions:u) mean max count
Regressions (primary) 0.8% 7.5% 40
Regressions (secondary) 1.2% 4.4% 29
Improvements (primary) -1.5% -6.0% 100
Improvements (secondary) -2.6% -21.6% 94
All (primary) -0.9% 7.5% 140

Overall the benefits here outweigh the negatives, with a number of good improvements. LLVM upgrades are something we’re going to keep rolling forward on regardless. For wall time metrics the improvement here looks particularly nice, with bootstrap times improved by ~4%, shaving 30 seconds of the total time measured by perf.

Nominated Issues

T-compiler

  • “Revert “Revert “Allow dynamic linking for iOS/tvOS targets.””” rust#100636
    • nominated by @davidtwco (comment)
    • ref: original PR rust#77716 reverting dynamic linking
    • bit of backstory with this dynamic linking on Apple platforms, see old meetings notes
    • even back to 2020 (see Zulip discussion)
    • T-compiler suggested to establish an MCP process to try avoiding this back and forth (see Wesley’s comment) which - to my knowledge - didn’t yet materialize for this PR
    • As pointed out by @davidtwco, this time the upcoming Cargo flag --crate-type could make things easier
  • “Add TyAlias into rustc_type_ir TyKind enum” rust#97974
    • It adds TyKind::TyAlias so that this type information can be used in rustdoc and for compiler errors. @GuillaumeGomez is currently blocked on finishing it because of a missing normalization and asked for help.

RFC

  • No nominated RFCs for T-compiler this time.

Next week’s WG checkins

  • @_WG-traits Types team by @nikomatsakis and @Jack Huey
  • @_WG-mir-opt MIR Optimizations by @oli