T Compiler Meeting Agenda 2022 09 01

T-compiler Meeting Agenda 2022-09-01

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 #[alias] attribute to allow symbol aliasing” compiler-team#526 (last review activity: about 41 days ago)
    • “Use RangeInclusive in SpanData instead of lo/hi” compiler-team#534 (last review activity: about 6 days ago)
    • “Allow informational -Z flags on stable compiler” compiler-team#542 (last review activity: about 6 days ago)
    • " Promote i586-unknown-linux-gnu to Tier 2 with Host Tools " compiler-team#543 (last review activity: about 6 days ago)
  • Pending FCP requests (check your boxes!)
    • No pending FCP requests this time.
  • Things in FCP (make sure you’re good with it)
    • “Add support for the LoongArch architecture” compiler-team#518
    • “Make format_args!() its own AST node (ast::ExprKind::FormatArgs)” compiler-team#541
    • “Stabilize raw-dylib for non-x86” rust#99916
    • “Consider #[must_use] annotation on async fn as also affecting the Future::Output” rust#100633
  • Accepted MCPs
  • Finalized FCPs (disposition merge)
    • “Tracking issue for RFC 2046, label-break-value” rust#48594
    • “Tracking Issue for “unsafe blocks in unsafe fn” (RFC #2585)” rust#71668
    • “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
    • “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
    • “relate closure_substs.parent_substs() to parent fn in NLL” rust#98835
    • “Strengthen invalid_value lint to forbid uninit primitives, adjust docs to say that’s UB” rust#98919
    • “Make forward compatibility lint deprecated_cfg_attr_crate_type_name deny by default” rust#99784
    • “Register wf obligation before normalizing in wfcheck " rust#100046

WG checkins

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

    Checkin text

  • @_WG-rls2.0 by @matklad (previous checkin):

    Checkin report from @Lukas Wirth The main things that have happened since the last time is that r-a has been changed from a submodule to a subtree in the rust-lang/rust repo, we now ship a rust-analyzer proc-macro-server component in rustup, and use this component from the sysroot as our proc-macro server by default when possible which circumvents the frequent proc-macro abi breakage problems. The coming sprint we are now looking into a way of getting rid of the RUSTC_WRAPPER hack in r-a in favor of a possible future command to build build-scripts and proc-macros only with cargo as well as trying to figure out a way to get analysis of crates.io dependencies of the standard library to work.

Backport nominations

T-compiler beta / T-compiler stable

  • No backport 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

  • (3 issues hidden since WIP or waiting on other teams)

Oldest PRs waiting for review

T-compiler

  • “Rewrite LLVM’s archive writer in Rust” rust#97485 (last review activity: 2 months ago)
    • rebased with additional changes from rust#98100. @bjorn3 can #97485 be closed?
  • “const_generics: correctly deal with bound variables” rust#98900 (last review activity: about 49 days ago)
    • PR is ready again for comments on new perf. results cc @lcnr @Jack Huey
  • “rustc_error, rustc_private: Switch to stable hash containers” rust#99334 (last review activity: about 44 days ago)
    • @oli unsure about the status. Comments needed from PR author @_Niklas Jonsson?
  • “Add support for MIPS VZ ISA extension” rust#99443 (last review activity: about 44 days ago)
    • rustbot assigned to @Esteban Küber. reroll?
  • “Add special_module_name lint” rust#94467 (last review activity: about 41 days ago)
    • Felix approved when tests pass. Anything else to do? (cc @bjorn3 )

Issues of Note

Short Summary

P-critical

T-compiler

  • “Wrong cast of u16 to usize on aarch64” rust#97463
    • PR #97800 is making progress
  • “Regression in consteval: error[E0080]: could not evaluate static initializer (unable to turn pointer into raw bytes)” rust#99923
    • PR #101101 has been merged
    • cc @RalfJ next step the blog post according to the plan outlined here?
  • “disable MIR inlining on beta-1.64” rust#101004
    • revert PR #101050 authored by Felix has been merged
    • issue can be closed?
  • “Regression of “mismatched types” error on trait method call with multiple candidates” rust#101066
    • PR to fix it rust#100966 waiting for review cc @pnkfelix - author @Michael Goulet (compiler-errors)

T-rustdoc

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

P-high regressions

P-high beta regressions

  • “Anon lifetime in impl Trait no longer suggests adding a lifetime parameter” rust#100615
    • (mentioned last week) @_TaKO8Ki self-assigned
  • “Borrow checking for static methods became more strict” rust#100725

Unassigned P-high nightly regressions

Performance logs

triage logs for 2022-08-30

A somewhat difficult week to triage due to the large amount of noise coming from two benchmarks. Hopefully this noise settles down in the future. Other than that, improvements much outweighed regressions with an average of 142 changes to instruction count averaging 0.7% improvement. There were no huge wins this week, however.

Triage done by @rylev. Revision range: 4a24f08b..0631ea5d

Summary:

(instructions:u) mean range count
Regressions (primary) 1.0% [0.2%, 2.6%] 4
Regressions (secondary) 1.3% [0.3%, 2.6%] 23
Improvements (primary) -0.7% [-2.8%, -0.2%] 138
Improvements (secondary) -1.3% [-2.7%, -0.2%] 71
All (primary) -0.7% [-2.8%, 2.6%] 142

2 Regressions, 3 Improvements, 10 Mixed; 6 of them in rollups 40 artifact comparisons made in total

Regressions

add depth_limit in QueryVTable to avoid entering a new tcx in layout_of #100748 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.3%, 0.7%] 8
Regressions (secondary) 0.7% [0.3%, 1.3%] 13
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.5% [0.3%, 0.7%] 8
  • Most of the regressions are happening in html5ever-0.26.0 and deeply-nested-multi which have been noisy lately. The regressions are small enough that it’s likely that we’re seeing that noise here too. Subsequent changes show improvements of the same magnitude reversing the regressions here.
  • However, there are some regressions that seem like they might be real, and they are all in doc profile test cases. The common query across the potentially real regressions is the build_impl query. This change seems like strictly less work, so I’m confused why this might be.
  • Left a comment asking if anyone had any good ideas despite the cachegrind run not revealing anything.

Don’t catch overflow when running with cargo doc #101039 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.6% [0.6%, 0.8%] 6
Regressions (secondary) 0.9% [0.2%, 1.3%] 9
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.6% [0.6%, 0.8%] 6
  • The primary regressions seem like noise (as they are reversed in the next perf run), but the secondary regressions seem like sustained regressions.
  • This was a fix for an issue that broke some crates so the minor perf hit in secondary benchmarks is likely acceptable even if it is real.

Improvements

Symbols: do not write string values of preinterned symbols into compiled artifacts #100803 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.6% [-2.5%, -0.2%] 35
Improvements (secondary) -1.7% [-2.6%, -0.3%] 24
All (primary) -0.6% [-2.5%, -0.2%] 35

Elide superfluous storage markers #99946 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.9% [0.9%, 0.9%] 1
Regressions (secondary) - - 0
Improvements (primary) -0.6% [-1.9%, -0.2%] 14
Improvements (secondary) -0.5% [-1.5%, -0.3%] 38
All (primary) -0.5% [-1.9%, 0.9%] 15

Rollup of 13 pull requests #101115 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.6% [-0.6%, -0.6%] 1
Improvements (secondary) -1.1% [-1.3%, -0.9%] 8
All (primary) -0.6% [-0.6%, -0.6%] 1

Mixed

Rollup of 15 pull requests #100963 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.2% [0.2%, 0.3%] 5
Regressions (secondary) 0.4% [0.2%, 0.6%] 17
Improvements (primary) -0.6% [-0.8%, -0.5%] 6
Improvements (secondary) -0.4% [-0.7%, -0.3%] 6
All (primary) -0.2% [-0.8%, 0.3%] 11
  • I looked for something obvious that might be causing this, and I couldn’t find anything promising.
  • It seems there are some PRs that are very likely not the cause. We can start by testing the others to see if they yield results.

Check projection types before inlining MIR #100571 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 0.9% [0.7%, 1.2%] 8
Improvements (primary) -0.4% [-1.1%, -0.2%] 115
Improvements (secondary) -0.4% [-1.3%, -0.1%] 42
All (primary) -0.4% [-1.1%, -0.2%] 115
  • All of the regressions are secondary, and many are in the recently noisy deeply-nested-multi. Additionally, the improvements far outweigh the regressions.
  • Marked as triaged

Rollup of 8 pull requests #101017 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.3% [0.3%, 0.3%] 2
Regressions (secondary) 1.5% [0.2%, 3.6%] 14
Improvements (primary) -0.4% [-0.6%, -0.3%] 8
Improvements (secondary) - - 0
All (primary) -0.3% [-0.6%, 0.3%] 10
  • Looks like #10034 is the likely culprit for a large part of the regressions
  • This is tracked by those working in the area.

Avoid reporting overflow in is_impossible_method #100705 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.7% [0.6%, 0.8%] 6
Regressions (secondary) 0.3% [0.3%, 0.3%] 1
Improvements (primary) -0.5% [-0.6%, -0.4%] 2
Improvements (secondary) - - 0
All (primary) 0.4% [-0.6%, 0.8%] 8
  • The primary perf regression on this PR seems to be reversed by #101039
  • Marked as triaged

Rollup of 8 pull requests #101037 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 1.5% [0.3%, 3.8%] 3
Regressions (secondary) 0.9% [0.5%, 1.2%] 7
Improvements (primary) - - 0
Improvements (secondary) -1.3% [-1.3%, -1.2%] 2
All (primary) 1.5% [0.3%, 3.8%] 3
  • Mostly a mixture of noisy and a small regression from #101006.
  • That PR is a correctness fix, so it seems likely that we’ll be ok with this small regression.

session: stabilize split debuginfo on linux #98051 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 0.2% [0.2%, 0.3%] 2
Improvements (primary) -0.6% [-0.7%, -0.5%] 6
Improvements (secondary) -0.9% [-1.1%, -0.6%] 6
All (primary) -0.6% [-0.7%, -0.5%] 6

interpret: remove support for uninitialized scalars #100043 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.8% [0.6%, 0.9%] 2
Regressions (secondary) 1.3% [0.3%, 3.2%] 18
Improvements (primary) -0.6% [-0.7%, -0.5%] 6
Improvements (secondary) - - 0
All (primary) -0.2% [-0.7%, 0.9%] 8
  • Looks like the primary regressions are due to noise.
  • The regressions in secondary benchmarks seem to be more real though. Looks like the most impacted query is eval_to_allocation_raw. Seems possible that that might indeed be impacted by this change (just going off the usage of eval_to_allocation_raw in const eval)?
  • Indeed, it was found that an unconditional format! was causing this issue.
  • This should be fixed by #101154.

Rollup of 9 pull requests #101064 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.6% [0.5%, 0.7%] 6
Regressions (secondary) 1.0% [1.0%, 1.0%] 1
Improvements (primary) - - 0
Improvements (secondary) -1.3% [-1.5%, -1.3%] 3
All (primary) 0.6% [0.5%, 0.7%] 6

Avoid cloning a collection only to iterate over it #100497 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.5%, 0.6%] 6
Regressions (secondary) 0.8% [0.5%, 1.1%] 10
Improvements (primary) -0.6% [-0.8%, -0.5%] 3
Improvements (secondary) -1.0% [-1.2%, -0.5%] 4
All (primary) 0.2% [-0.8%, 0.6%] 9
  • The regressions seem to just be noise. The improvements though seem real. See here.

Rollup of 8 pull requests #101152 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.6% [0.5%, 0.7%] 10
Regressions (secondary) - - 0
Improvements (primary) -0.3% [-0.5%, -0.2%] 24
Improvements (secondary) -0.7% [-1.9%, -0.3%] 34
All (primary) -0.0% [-0.5%, 0.7%] 34
  • #99821 is responsible for all the improvements and regressions for this rollup.
  • Given that the improvements outweigh the regressions we mark this as triaged.

Nominated Issues

T-compiler

  • No nominated issues for T-compiler this time.

RFC

  • No nominated RFCs for T-compiler this time.

Next week’s WG checkins

  • @_WG-self-profile @mw and @Wesley Wiser