T Compiler Meeting Agenda 2021 10 28

T-compiler Meeting Agenda 2021-10-28

Tracking Issue

Announcements

  • 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 Felix or Wesley so we can pass it along). “If you see something, say something”

MCPs/FCPs

  • New MCPs (take a look, see if you like them!)
    • No new proposals this time.
  • Old MCPs (not seconded, take a look)
    • “CI should exercise (subset of) tests under –stage 1” compiler-team#439 (last review activity: GH none, Zulip about 10 days 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 none, Zulip 1 month ago)
    • “Make -Z binary-dep-depinfo the default behavior” compiler-team#464 (last review activity: GH none, Zulip about 1 month ago)
  • Pending FCP requests (check your boxes!)
    • “Tracking issue for #![feature(const_precise_live_drops)]” rust#73255
    • “Stabilize -Z strip as -C strip” rust#90058
    • “Stabilize -Z symbol-mangling-version as -C symbol-mangling-version” rust#90128
  • Things in FCP (make sure you’re good with it)
    • “Write text output files to stdout if options like -o - or --emit asm=- are provided” compiler-team#431
  • Accepted MCPs
    • “Tier 3 target proposal: x86_64-unknown-none (freestanding/bare-metal x86-64)” compiler-team#462
  • Finalized FCPs (disposition merge)
    • No new finished FCP (disposition merge) this time.

WG checkins

  • @WG-mir-opt checkin by @oli (previous checkin):

    We have moved to a “merge fast” policy for new mir opts. This means we add them behind mir-opt-level=4 (very unstable, probably unsound) without necessarily reviewing them with the detail required for anything we expose to users. This allows us to quickly move ahead with new opts without burdening the author with too much rebasing due to mir tests changing under them.

    #90016 is an optimization currently WIP that uses dataflow to figure out when a match arm is unreachable. This is a very simple form of VRA (value range analysis) that only works for matches on integers, bools and chars, but it is expected to become more powerful and be able to handle enums and comparisons, too.

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

    There’s a polymorphization update! Surprise!

    • @_davidtwco submitted #89426 which makes polymorphization pass tests on master, removes some special-case predicate logic that isn’t necessary now (after earlier work on the v0 mangling scheme that landed many months ago), and makes the unused_generic_params query work on instances rather than just functions (so that shims can be polymorphized). Approved and merged by @_lcnr.
    • @lcnr is working on a fix for #75325, this is the primary blocker for polymorphization, and the fix should enable greater polymorphization in the long run. Work-in-progress PR at #90057, and some rough sketches of the design in a HackMD document.

Backport nominations

T-compiler stable / T-compiler beta

  • :beta: “Fix ICE when forgetting to Box a parameter to a Self::func call” rust#90221
    • Closes a P-high issue rust#90213
    • (Unilaterally approved by @pnkfelix, no discussion required.)
  • No stable nominations for T-compiler this time.

T-rustdoc stable / T-rustdoc beta

  • :beta: “Fix documentation header sizes” rust#90186
    • no concerns from T-rustdoc, can be safely beta-backported
  • No stable nominations for T-rustdoc this time.

:back: / :shrug: / :hand:

PRs S-waiting-on-team

T-compiler

  • “Change default panic strategy to abort for wasm32-unknown-emscripten” rust#89762
    • also nominated by @pnkfelix (see comment) to discuss the strategy to adopt
    • Fixes a P-high issue rust#85821, emscripten target compiling on trivial code

Oldest PRs waiting for review

T-compiler

  • “Fixes incorrect handling of ADT’s drop requirements” rust#90218
    • fixes a P-high occurring when trying to migrate code to 2021 edition
    • (pnkfelix askes during meeting: this shouldn’t be on this list)
  • “Support #[global_allocator] without the allocator shim” rust#86844 (last review activity: 2 months ago)
    • seems to need a reviewer assigned
    • review from T-lang or T-compiler?
  • “Turn TrapUnreachable off by default” rust#88826
    • seems to need a reviewer assigned
  • “Allow use of AddressSanitizer on Windows by linking to existing libraries” rust#89369
  • “debuginfo: Add script for Rust support in lldb-mi” rust#89163 (last review activity: 19 days ago)
  • “Implement concat_bytes!” rust#87599 (last review activity: 3 months ago)
  • “Make specifying repr optional for fieldless enums” rust#88203 (last review activity: 2 months ago)
    • review seems to point out there’s some more work to do, see comment

Issues of Note

Short Summary

P-critical

T-compiler

  • “cargo fails to build on Windows with nightly” rust#90019
    • assigned to @mw, re-opened to track beta-backport (beta-accepted)

T-rustdoc

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

P-high regressions

P-high beta regressions

  • “Incremental compilation fails in all cases on SystemZ (s390x)” rust#90123
    • incremental compile ICE on Tier 2 platform
    • There seems to be a patch on the way to fix (see comment)
  • “regression: rustc suggests .as_ref() at incorrect location and other spans have regressed” rust#90286
    • assigned P-high as it could be confusing for beginners
  • “DWARF info for static vars in lib crates has stopped being produced reliably in LTO builds” rust#90357
    • needs an assignee
    • bisection seems to point to PR rust#89041 (cc: @nagisa)

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
    • opened by @XAMPPRocky
    • issue seems to receive mentions from other issues
    • @Hans Kratz comments probably related to PR rust#83655

Performance logs

triage logs for 2021-10-26

Multiple regressions this week, several of which were in rollups, without much to balance them out on the improvements front.

Triage done by @simulacrum. Revision range: d45ed7502ad225739270a368528725930f54b7b6..3c8f001d454b1b495f7472d8430ef8fdf10aac11

5 Regressions, 4 Improvements, 3 Mixed; 3 of them in rollups; 35 comparisons made in total

Regressions

resolve: Use NameBinding for local variables and generic parameters #89100

  • Very large regression in instruction counts (up to 99.5% on incr-unchanged builds of style-servo)
  • Reverted in #90130.

Rollup of 6 pull requests #90235

  • Very large regression in instruction counts (up to 9.8% on incr-full builds of deeply-nested-async)
  • Probably caused by new compiler-internal lint (#89558), which appears to be run on end-user code (despite being allow-by-default). Suggested a few possible fixes or a revert if we can’t do so quickly.

Inline CStr::from_ptr #90007

  • Small regression in instruction counts (up to 0.4% on incr-unchanged builds of helloworld)
  • Regression limited to stress tests and fairly minor. Seems likely to be a litle extra work in codegen, as the regressions are all in -opt builds. This change is done to permit better optimization, skipping a call to strlen in some cases, so seems worthwhile.

Implement coherence checks for negative trait impls #90104

  • Small regression in instruction counts (up to 0.6% on full builds of diesel)
  • Looks like a real regression, but the feature is important and the regression is relatively small.

Rollup of 5 pull requests #90203

  • Moderate regression in instruction counts (up to 0.6% on full builds of externs)
  • Regression limited to rustdoc, likely due to the addition of code-scraping from the examples directory. Does not seem major enough to warrant deep investigation, but have left a comment on the likely PR.

Improvements

  • Adopt let_else across the compiler #89933
  • Revert “resolve: Use NameBinding for local variables and generic parameters” #90130
  • Specialize HashStable for [u8] slices #90208
  • Build the query vtable directly. #90210

Mixed

Rollup of 10 pull requests #90067

  • Small improvement in instruction counts (up to -1.4% on incr-patched: println builds of coercions)
  • Moderate regression in instruction counts (up to 1.1% on incr-patched: b9b3e592dd cherry picked builds of style-servo)
  • Left a comment with a few suggestions, but the regression and improvements seem both major and without obvious cause.

Merge the two depkind vtables #89978

  • Moderate improvement in instruction counts (up to -2.0% on incr-unchanged builds of helloworld)
  • Moderate regression in instruction counts (up to 0.9% on incr-unchanged builds of clap-rs)
  • Improvements mostly outweigh the regressions

Make new symbol mangling scheme default for compiler itself. #90054

  • Moderate improvement in instruction counts (up to -0.8% on incr-unchanged builds of deeply-nested-async)
  • Moderate regression in instruction counts (up to 0.4% on incr-unchanged builds of deep-vector)
  • Mostly improvements, and digging in is hard since tools like rustfilt have slightly different output across the symbol mangling boundary. Regressions seem limited to a just a few benchmarks and are small enough that this seems acceptable.

Untriaged Pull Requests

Nominated Issues

T-compiler

  • “Add new tier 3 target: x86_64-unknown-none” rust#89062
  • “sess: default to v0 symbol mangling” rust#89917
    • PR opened by @davidtwco
    • @mw nominated to discuss @wesley wiser suggestion
  • “Enable verification for 1/32th of queries loaded from disk” rust#90361
    • PR opened by @simulacrum
    • PR should fix rust#85783
    • nominated by @mw (comment) to evaluate the perf regression and possible new fingerprint errors

RFC

  • No nominated RFCs for T-compiler this time.