T Compiler Meeting Agenda 2021 11 24

T-compiler Meeting Agenda 2021-11-24

Tracking Issue

Announcements

  • :loudspeaker: next week December, 2nd, release of Rust stable 1.57
  • 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).

MCPs/FCPs

  • New MCPs (take a look, see if you like them!)
    • “Tier 3 target proposal: riscv64gc-linux-android (Android target for riscv64gc)” compiler-team#472
  • Old MCPs (not seconded, take a look)
    • “CI should exercise (subset of) tests under –stage 1” compiler-team#439 (last review activity: 3 months 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: 4 months ago)
    • “Make -Z binary-dep-depinfo the default behavior” compiler-team#464 (last review activity: about 54 days ago)
  • Pending FCP requests (check your boxes!)
    • No pending FCP requests this time.
  • Things in FCP (make sure you’re good with it)
    • “Unstable lints should be considered unknown” compiler-team#469
    • “Tracking Issue for cargo report future-incompat” rust#71249
    • “Tracking Issue for inline assembly (asm!)” rust#72016
  • Accepted MCPs
    • No new accepted proposals this time.
  • Finalized FCPs (disposition merge)
    • No new finished FCP (disposition merge) this time.

WG checkins

nothing to report at this time

  • @_WG-rustc-dev-guide by @Santiago Pastorino and @Yuki Okushi|217081 (previous checkin):

Most notable changes

  • Make subtree docs an overview and add link to clippy docs for details #1244
  • Describe drop elaboration #1240
  • Edit bootstrapping chapter #1239
  • Add documentation for LLVM CFI support #1234

Most notable WIPs

  • Document more compiletest headers. #1249
  • Added detail to codegen section #1216
  • Document inert vs active attributes #1110
  • Explain the new valtree system for type level constants. #1097

Backport nominations

T-compiler stable / T-compiler beta

  • :beta: “relate lifetime in TypeOutlives bounds on drop impls” rust#90840
    • fixes p-critical unsoundness rust#90838
    • @pnkfelix has also produced a scope-limited backport in rust#91136
    • @Jack Huey comments on rust#91136 on which of the two is better suited for a backport
  • :stable: “relate lifetime in TypeOutlives bounds on drop impls” rust#90840
    • see beta nomination

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

  • No PRs waiting for T-compiler

Oldest PRs waiting for review

T-compiler

Issues of Note

Short Summary

P-critical

T-compiler

  • “Miscompilation where binding only some fields leaks the others” rust#90752
  • “Unsound drop due to imperfect lifetime checks” rust#90838
  • “Implied bounds by associated types as function parameters are inconsistent in an unsound way.” rust#91068
  • “Huge compile-time regression in beta/nightly” rust#91128
    • we have a reproducible example
    • seems correlated to the new LLVM pass-manager (disabling it solves the issue)
    • (the new LLVM pass-manager will enter in stable and is enabled by default)

T-rustdoc

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

P-high regressions

P-high beta regressions

  • “Compilation appears to loop indefinitely " rust#89195
    • discussed last week
      • @pnkfelix self assigned as reminder to reach out to Kyle Huey and schedule chat with @Aaron Hill
  • “regression: rustc suggests .as_ref() at incorrect location and other spans have regressed” rust#90286
    • @Esteban Küber comments wrt to a revert that it would cause another bug to resurface, would try to prepare a fix before the next release
  • “DWARF info for static vars in lib crates has stopped being produced reliably in LTO builds” rust#90357
    • bisection seems to point to rust#89041 but cuviper suggests could be a dupe of rust#90301 and #89041 possibly just exacerbated the LLVM issue
    • also reproduces on nightly

Unassigned P-high nightly regressions

Performance logs

triage logs 2021-11-23

This week, there were a number of cases where the incr-unchanged variants of inflate went up or down by 5% to 6%; we believe these are instances of increased noise in benchmarks documented on rustc-perf#1105. I was tempted to remove these from the report, but its non-trivial to re-construct the report “as if” some benchmark were omitted.

Otherwise, there were some nice wins for performance. For example, PR #90996 more than halved the compilation time for full builds of diesel by revising how we hash ObligationCauseData. If anyone is interested, it might be good to follow-up on the effects of PR #90352, “Simplify for loop desugar”, where we have hypothesized that the increased compilation time is due to more LLVM optimizations being applied.

Triage done by @pnkfelix. Revision range: 934624fe..22c2d9dd

1 Regressions, 3 Improvements, 8 Mixed; 3 of them in rollups, 30 Untriaged PRs 34 comparisons made in total

Regressions

rustdoc: Make two small cleanups #91073

  • Very large regression in instruction counts (up to 5.3% on incr-unchanged builds of inflate)
  • This was previously triaged as spurious by the PR author.
  • (Only inflate debug incr-unchanged was affected significantly.)

Improvements

  • Rollup of 7 pull requests #90966
  • Optimize impl Hash for ObligationCauseData by not hashing ObligationCauseCode variant fields #90996
  • Avoid documenting top-level private imports #91094
  • rustdoc: Cleanup DocFragment #91034
  • libcore: assume the input of next_code_point and next_code_point_reverse is UTF-8-like #89611

Mixed

Remove DropArena. #90919

std: Get the standard library compiling for wasm64 #90382

  • Large improvement in instruction counts (up to -4.3% on incr-unchanged builds of clap-rs)
  • Moderate regression in instruction counts (up to 1.0% on incr-unchanged builds of issue-88862)
  • There were a lot of various changes in this PR, such as updates to dependencies (compiler_builtins and dlmalloc). We probably shoud have done a pre-merge rust-timer run on this PR.
  • The flagged regressions of magnitude greater than 0.5% are isolated to “issue-88862 check” and “style-servo check”. The improvements tended to outweigh the regressions. For the most part almost all significant performance effects are isolated to check builds.

Rollup of 8 pull requests #91019

  • Moderate improvement in instruction counts (up to -0.6% on incr-unchanged builds of style-servo)
  • Moderate regression in instruction counts (up to 0.8% on full builds of await-call-tree)
  • The regression is entirely associated with doc builds, which led to the PR author to flag PR #90750 as the root cause.
  • It seems to me like the extra work injected by PR #90750 may be unavoidable; but was it expected to be significant?

Implement clone_from for State #90535

  • Small improvement in instruction counts (up to -0.2% on full builds of many-assoc-items)
  • Very large regression in instruction counts (up to 6.1% on incr-unchanged builds of inflate)
  • This was evaluated for its effect on performance prior to merge; that run returned no relevant changes.
  • As noted elsewhere, for this report we should probably treat “inflate” as noisy.

Update stdarch #91052

  • Very large improvement in instruction counts (up to -5.8% on incr-unchanged builds of inflate)
  • Small regression in instruction counts (up to 0.3% on incr-patched: println builds of html5ever)
  • The only benchmark that regressed is “html5ever debug” (“incr-patched: println” and “incr-unchanged”), and only by a relatively small amount. This seems acceptable to me, compared to the effort involved in figuring out how this change could be related to that effect.

Point at source of trait bound obligations in more places #89580

  • Large improvement in instruction counts (up to -5.0% on incr-unchanged builds of inflate)
  • Small regression in instruction counts (up to 1.1% on incr-unchanged builds of wg-grammar)
  • (Again, we can probably ignore the change to inflate.)
  • Other than that, there were a broad set of small regressions. Putting aside the ones tagged with ? (“noisy”), there are 19 benchmarks that regressed by 0.10% to 0.42%. This seems like an acceptable cost.

Simplify for loop desugar #90352

  • Very large improvement in instruction counts (up to -6.2% on incr-full builds of clap-rs)
  • Large regression in instruction counts (up to 2.3% on full builds of regex)
  • This was triaged in the PR comments both before it landed and after it landed with the justification “The regressions seem to all be in -opt builds and solely part of the time spent in LLVM, so I’m hoping it’s that more optimizations apply now (and worst case some optimizations require more work but don’t result in better code).”

Manually outline error on incremental_verify_ich #89883

  • Small improvement in instruction counts (up to -0.8% on incr-unchanged builds of clap-rs)
  • Large regression in instruction counts (up to 1.1% on incr-unchanged builds of coercions)
  • The pre-merge rustc-timer run did not predict such a significant impact on coercions.
  • The driving force for this change was to reduce the critical path in bootstrap time, so the most important thing to look at is the bootstrap timing data. Specifically: while there is a big mix of ups and downs on the percentages column, the crate that takes the longest to compile (rustc_query_impl, the laggard at over 80 seconds of compilation time).
  • This PR brings the compilation time of rustc_query_impl from 87.1 seconds down to 85.6 seconds, a -1.8% improvement.
  • That is consistent with the predicted effect of the PR, and justifies the isolated impact on instruction counts.

Nominated Issues

T-compiler

  • “Tracking issue for plugin stabilization (plugin, plugin_registrar features)” rust#29597
  • “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
  • “Miscompilation where binding only some fields leaks the others” rust#90752
    • @tmandry nominated
    • great writeup from @Dylan MacKenzie (ecstatic-morse), suggests a quick fix
    • Opened PR rust#90788 that implements the quick fix suggested. PR is beiung reviewed.
    • wg-prio assigned P-critical, but being a regression from stable probably not a release blocker?

RFC

  • No nominated RFCs for T-compiler this time.