T Compiler Meeting Agenda 2021 11 11

T-compiler Meeting Agenda 2021-11-11

Tracking Issue

Announcements

  • Reminder: if you see a PR/issue that seems like there might be legal implications due to copywrite/IP/etc, please let the Core team know (or at least message @pnkfelix or @Wesley Wiser so we can pass it along).
    • A new PR has been filed to include the Playstation Vita as tier 3 target. This comment suggests the situation could be similar to other end-of-life proprietary platforms. @Josh Triplett provided tips for an MCP
  • @Hans Kratz is gathering interest around the macOS target platform and build a notification group who can respond to issues, see Zulip thread.

MCPs/FCPs

  • New MCPs (take a look, see if you like them!)
  • Old MCPs (not seconded, take a look)
    • “CI should exercise (subset of) tests under –stage 1” compiler-team#439 (last review activity: GH 2 months ago, zulip 1 month ago)
    • “Accept pc in place of unknown and unknown in place of pc for x86_64 and i?86 targets” compiler-team#441 (last review activity: GH 4 months ago, zulip 40 days)
    • “Make -Z binary-dep-depinfo the default behavior” compiler-team#464 (last review activity: about 41 days ago)
  • Pending FCP requests (check your boxes!)
    • “Tracking Issue for cargo report future-incompat” rust#71249
    • “Tracking Issue for inline assembly (asm!)” rust#72016
    • “Tracking issue for #![feature(const_precise_live_drops)]” rust#73255
  • Things in FCP (make sure you’re good with it)
    • “Create a macos notification group” compiler-team#470
    • “Stabilize -Z strip as -C strip” rust#90058
    • “Stabilize -Z symbol-mangling-version=v0 as -C symbol-mangling-version=v0” rust#90128
  • Accepted MCPs
    • “Tier 3 target proposal: x86_64-unknown-none (freestanding/bare-metal x86-64)” compiler-team#462
  • Finalized FCPs (disposition merge)
    • “Write text output files to stdout if options like -o - or --emit asm=- are provided” compiler-team#431
    • “Tracking Issue for destructuring_assignment” rust#71126
    • “Tracking Issue for relaxed struct unsizing rules” rust#81793
    • “GATs: Decide whether to have defaults for where Self: 'a” rust#87479
    • “Stabilize const_raw_ptr_deref for *const T” rust#89551

WG checkins

@WG-self-profile from @mw and @Wesley Wiser (previous checkin):

The compiler can record artifact sizes (e.g. size of object files, rlibs, incr. comp. cache, etc) now and will do so by default when invoking it with -Zself-profile. The summarize tool will display these artifact sizes in a separate table. @rylev is working hard on also showing them on perf.rlo – which turned out to be a little more tricky than expected but we’ll get there :)This will be very useful for gauging the impact a PR might have on binary sizes and the like.

Backport nominations

T-compiler stable / T-compiler beta

  • :beta: “Apply adjustments for field expression even if inaccessible” rust#90508
    • fixes a P-medium rust#90483
    • nominated by @davidtwco since it’s a regression on stable (comment)
  • :beta: “Properly register text_direction_codepoint_in_comment lint.” rust#90626
  • :beta: “Warn for variables that are no longer captured” rust#90597
  • :stable: “Warn for variables that are no longer captured” rust#90597
    • see beta nomination

T-rustdoc stable / T-rustdoc beta

  • :beta: “Show all Deref implementations recursively” rust#90183
    • discussed last week
    • waiting for feedback from T-rustdoc
  • :beta: “rustdoc: Go back to loading all external crates unconditionally” rust#90489
  • :stable: “rustdoc: Go back to loading all external crates unconditionally” rust#90489
    • same as above

:back: / :shrug: / :hand:

PRs S-waiting-on-team

T-compiler

  • “Wrap libraries in linker groups, allowing backwards/circular references” rust#85805
    • Discussed a while ago
    • @Josh Triplett will rework this patch to provide a separate option that enables linker groups (comment)
    • Can S-waiting-on-team be removed?
  • “Change default panic strategy to abort for wasm32-unknown-emscripten” rust#89762
    • mentioned in a previous meeting, seems it can move forward?
    • Can S-waiting-on-team be removed?

Oldest PRs waiting for review

T-compiler

  • “Add codegen option for branch protection and pointer authentication on AArch64” rust#88354 (last review activity: 2 months ago)
    • last comment is paging @nagisa
  • “Check for duplicate attributes.” rust#88681 (last review activity: about 59 days ago)
    • last comment is paging @Vadim Petrochenkov but review can be reassigned
  • “Makes docs for references a little less confusing” rust#88361 (last review activity: none)
    • rust-highfive assigned to @Josh Triplett but review can be reassigned
  • “Suggest i += 1 when we see i++ or ++i” rust#88672 (last review activity: 2 months ago)
  • “asm/arm: allow r9 when usable, make diagnostics more specific” rust#88879 (last review activity: about 57 days ago)
    • last comment is paging @Amanieu

Issues of Note

Short Summary

P-critical

T-compiler

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

T-rustdoc

  • “Index out of bounds when running cargo doc in rustc_metadata/src/creader.rs” rust#84738
    • old issue reprioritized as P-critical by T-rustdoc
    • since 1.56 it breaks doc builds for some crates) (note: this comment)
    • under scrutiny by T-rustdoc

P-high regressions

P-high beta regressions

  • “Incremental compilation fails in all cases on SystemZ (s390x)” rust#90123
    • going to be closed by rust#90403 (beta-accepted)
  • “regression: rustc suggests .as_ref() at incorrect location and other spans have regressed” rust#90286
    • confusing wrong diagnostic
    • @pnkfelix self-assigned
  • “DWARF info for static vars in lib crates has stopped being produced reliably in LTO builds” rust#90357
    • @pnkfelix self-assigned
  • “warn(must_not_suspend) started being raised incorrectly when moving from stable to nightly” rust#90459
    • currently unassigned
    • should be fixed by rust#89826 (it’s on master and beta-accepted)

Unassigned P-high nightly regressions

  • “Undefined reference to getauxval in function init_have_lse_atomics when compiling to nightly aarch64-unknown-linux-musl” rust#89626

Performance logs

triage logs for 2021-11-09

Largely a positive week despite taking a significant performance hit from turning on incremental compilation verification for a subsection of the total queries that the compiler does in order to more quickly catch bugs in incremental compilation. Luckily optimizations in bidi detection brought large performance improvements.

Triage done by @rylev. Revision range: 6384dca100f3cedfa031a9204586f94f8612eae5..eee8b9c7bafade55981d155dae71657f1cc55a22

2 Regressions, 4 Improvements, 4 Mixed; 1 of them in rollups 29 Untriaged Pull Requests 45 comparisons made in total

Regressions

Implement RefUnwindSafe for Rc<T> #87467

  • Large regression in instruction counts (up to 1.1% on full builds of cargo)
  • Unsurprising regression in one instance of a rustdoc run since get_auto_trait_impls has bad algoritmic complexity. This issue is being tracked somewhat here.

Enable verification for 1/32th of queries loaded from disk #90361

  • Very large regression in instruction counts (up to 6.3% on incr-unchanged builds of coercions)
  • Allows for verification of incremental compilation results to more easily catch bugs (which have unfortunately been a bit too common in the past).
  • As noted in the PR this is a regression of at most 7% on coercions opt incr-unchanged, and typically less than 0.5% on other benchmarks (largely limited to incr-unchanged).
  • The PR author and reviewer reviewed the regression impact and it was deemed acceptable.

Improvements

  • Optimize bidi character detection. #90559
  • Fix ICE when rustdoc is scraping examples inside of a proc macro #90583
  • Replace some uses of vec.drain(..) with vec.into_iter() #90655
  • rustdoc: Cleanup clean::Impl and other parts of clean #90675

Mixed

rustdoc: Add DocVisitor and use it where possible #90475

  • Moderate improvement in instruction counts (up to -0.8% on full builds of serde)
  • Small regression in instruction counts (up to 0.5% on full builds of webrender)
  • The regressions have already been justified, but in short the improvements outweigh the regressions, and the code is much cleaner as a result.

Improve error when an .rlib can’t be parsed #88368

  • Moderate improvement in instruction counts (up to -0.3% on full builds of externs)
  • Small regression in instruction counts (up to 0.5% on incr-unchanged builds of helloworld)
  • It was pointed out that this could be simply due to a different binary layout impacting performance.
  • When taking into account significance factor, the improvements outweigh the regressions, and so it’s likely not worth it to investigate deeper.

Rollup of 4 pull requests #90695

  • Moderate improvement in instruction counts (up to -0.5% on full builds of deeply-nested-async)
  • Small regression in instruction counts (up to 0.3% on incr-unchanged builds of html5ever)
  • Nothing jumps out as a possible cause for this but luckily the perf changes in question are small.

Don’t destructure args tuple in format_args! #90485

  • Large improvement in instruction counts (up to -4.0% on incr-unchanged builds of inflate)
  • Large regression in instruction counts (up to 1.3% on incr-patched: println builds of html5ever)
  • The regressions seem to be coming in expand_crate which might be impacted by this change. However, nothing stands out immediately as a definite cause for concern.
  • Left a comment for the author and reviewer to ask if they have an ideas before we investigate more.

Nominated Issues

T-compiler

T-compiler

  • “Tracking issue for plugin stabilization (plugin, plugin_registrar features)” rust#29597
    • nominated by @Josh Triplett, asks in comment: “what’s still using plugin support?”
  • “Change char type in debuginfo to DW_ATE_UTF” rust#89887
  • “sess: default to v0 symbol mangling” rust#89917
  • “implement aspect-oriented programming (AOP) for Rust” rust#90721
    • @mw suggested to first file an MCP
    • @davidtwco is a colleague of the author at Huawei and can coordinate between the compiler team and the author where required.
    • Does this might be of interest for T-lang?
  • “Miscompilation where binding only some fields leaks the others” rust#90752
    • @tmandry nominated
    • great writeup from @Dylan MacKenzie (ecstatic-morse)

RFC

  • No nominated RFCs for T-compiler this time.