T-compiler Meeting Agenda 2024-08-08

Announcements

  • A Rust 1.80.1 will be out (with stable backports from last week): blog post.
  • Reminder: if you see a PR/issue that seems like there might be legal implications due to copyright/IP/etc, please let us know (or at least message @davidtwco or @Wesley Wiser so we can pass it along).

Other WG meetings

MCPs/FCPs

WG checkins

None

Backport nominations

T-compiler beta / T-compiler stable

  • :beta: "Do not apply #[do_not_recommend] if the feature flag is not set" rust#128674
    • Authored by weiznich
    • nominated preemptively (still a work in progress)
  • :beta: "Normalize when equating dyn tails in MIR borrowck" rust#128694
    • Authored by compiler-errors
    • Fixes #128621 a P-high regression
  • No stable nominations for T-compiler this time.

T-types stable / T-types beta

  • No beta nominations for T-types this time.
  • No stable nominations for T-types this time.

PRs S-waiting-on-team

T-compiler

Issues of Note

Short Summary

P-critical

T-compiler

  • "ICE: adding a def'n for node-id NodeId(18) and def kind AnonConst but a previous def'n exists" rust#128016
    • Can be fixed by #128760 in beta
    • Note: that patch points to rust:beta branch but it is not beta-nominated. Should it be?

T-types

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

P-high regressions

P-high beta regressions

  • No P-high beta regressions this time.

Unassigned P-high nightly regressions

  • "Generated WebAssembly unexpectedly requires reference types" rust#128475
    • Alex Chrichton prepared #128511 to document this behaviour. "Fixing" this would very complicated and involves LLVM.

Performance logs

triage logs 2024-08-06

This week saw several large improvements caused mostly by the update to LLVM 19. There were some regressions in several pull requests, but most of them were immediately fixed in a follow-up PR.

Triage done by @kobzol.
Revision range: 7e3a9718..8c7e0e16

Summary:

(instructions:u) mean range count
Regressions (primary) 1.0% [0.2%, 3.8%] 91
Regressions (secondary) 1.9% [0.2%, 19.2%] 104
Improvements (primary) -4.4% [-15.8%, -0.3%] 120
Improvements (secondary) -3.3% [-10.4%, -0.2%] 70
All (primary) -2.1% [-15.8%, 3.8%] 211

6 Regressions, 3 Improvements, 5 Mixed; 4 of them in rollups
51 artifact comparisons made in total

Regressions

Rollup of 7 pull requests #128413 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.6% [0.2%, 1.9%] 28
Regressions (secondary) 0.3% [0.2%, 0.4%] 7
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.6% [0.2%, 1.9%] 28

Rollup of 6 pull requests #128469 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.3% [0.2%, 0.7%] 10
Regressions (secondary) 1.0% [0.2%, 2.1%] 36
Improvements (primary) -0.2% [-0.2%, -0.2%] 1
Improvements (secondary) -2.5% [-2.5%, -2.5%] 1
All (primary) 0.3% [-0.2%, 0.7%] 11

Rewrite binary search implementation #128254 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.5% [0.2%, 1.7%] 16
Regressions (secondary) 6.2% [0.2%, 19.5%] 6
Improvements (primary) - - 0
Improvements (secondary) -4.1% [-4.1%, -4.1%] 1
All (primary) 0.5% [0.2%, 1.7%] 16
  • This PR optimized the standard library implementation of binary search.
  • The regression is caused by LLVM spending more time in optimizing the new binary search, because it is now more amenable to unrolling and other optimizations.
  • Marked as triaged.

Rollup of 7 pull requests #128614 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 0.8% [0.4%, 1.3%] 10
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) - - 0

Update the stdarch submodule #128466 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.7% [0.2%, 2.6%] 13
Regressions (secondary) 1.0% [0.2%, 4.3%] 32
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.7% [0.2%, 2.6%] 13
  • Small regression on several benchmarks, caused by the standard library becoming larger.
  • Marked as triaged.

Enforce supertrait outlives obligations hold when confirming impl #124336 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.6% [0.2%, 1.5%] 95
Regressions (secondary) 1.8% [0.2%, 4.2%] 38
Improvements (primary) - - 0
Improvements (secondary) - - 0
All (primary) 0.6% [0.2%, 1.5%] 95
  • Medium regression on a lot of primary benchmarks.
  • Not marking as triaged yet, investigation ongoing.

Improvements

Rollup of 6 pull requests #128504 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) - - 0
Improvements (secondary) -0.4% [-0.5%, -0.3%] 12
All (primary) - - 0

Delegation: second attempt to improve perf #128441 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.4% [-0.7%, -0.2%] 9
Improvements (secondary) -0.5% [-1.1%, -0.1%] 9
All (primary) -0.4% [-0.7%, -0.2%] 9

Check divergence value first before doing span operations in warn_if_unreachable #128544 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) - - 0
Improvements (primary) -0.3% [-0.5%, -0.2%] 21
Improvements (secondary) -1.0% [-2.1%, -0.2%] 34
All (primary) -0.3% [-0.5%, -0.2%] 21

Mixed

Delegation: support generics for delegation from free functions #125929 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.4% [0.2%, 0.8%] 11
Regressions (secondary) 0.5% [0.1%, 1.2%] 10
Improvements (primary) - - 0
Improvements (secondary) -0.3% [-0.4%, -0.2%] 2
All (primary) 0.4% [0.2%, 0.8%] 11

Update to LLVM 19 #127513 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 1.3% [0.2%, 3.6%] 13
Regressions (secondary) 0.9% [0.2%, 3.4%] 37
Improvements (primary) -3.2% [-15.8%, -0.2%] 172
Improvements (secondary) -2.6% [-10.5%, -0.2%] 82
All (primary) -2.9% [-15.8%, 3.6%] 185
  • This was a large performance increase caused by an update to LLVM 19.
  • Improvement far outweigh the regressions.
  • Marked as triaged.

Accelerate GVN a little #126991 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) 0.3% [0.3%, 0.3%] 1
Regressions (secondary) 2.1% [2.1%, 2.1%] 1
Improvements (primary) -0.4% [-0.4%, -0.3%] 2
Improvements (secondary) -0.7% [-1.0%, -0.4%] 6
All (primary) -0.2% [-0.4%, 0.3%] 3
  • More improvements than regressions.
  • Marked as triaged.

Revert recent changes to dead code analysis #128404 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 1.1% [0.7%, 1.7%] 9
Improvements (primary) -0.4% [-1.2%, -0.2%] 47
Improvements (secondary) -2.9% [-10.7%, -0.2%] 15
All (primary) -0.4% [-1.2%, -0.2%] 47
  • More improvements than regressions.
  • Marked as triaged.

Change output normalization logic to be linear against size of output #128200 (Comparison Link)

(instructions:u) mean range count
Regressions (primary) - - 0
Regressions (secondary) 0.4% [0.3%, 0.5%] 6
Improvements (primary) -0.9% [-2.6%, -0.2%] 19
Improvements (secondary) -0.5% [-0.7%, -0.3%] 10
All (primary) -0.9% [-2.6%, -0.2%] 19

Nominated Issues

T-compiler

  • "ill-typed unused FFI declarations can cause UB" rust#46188
    • nominated by @RalfJ (comment)
    • Attempted to write a patch in #128247 but ultimately hard to fix (related Zulip topic). I am also increasingly skeptical whether LLVM will cope well with functions declared as @func_name = external global [0 x i8] – aren't functions special e.g. in how they are treated by the linker on some platforms, and other things like that? Also this will hit entirely untested codepaths in LLVM.
    • How to best resolve this issue? Re-consider priority?
  • "Tracking Issue for the C-cmse-nonsecure-call ABI" rust#81391
    • nominated by @Jieyou Xu (comment):
    • also nominated for T-lang
    • Does this need an MCP/FCP for stabilization, or does this need further design?
    • Does this need a joint T-compiler/T-lang FCP?
    • We should help the people working on cmse-related things to find knowledgeable reviewers / domain experts who can help reviewing the changes, or otherwise provide advice on how to split related PRs for easier reviewing.
    • Are cmse-related efforts being tracked anywhere?
  • "[DRAFT] #[contracts::requires(…)]" rust#128045
    • nominated by @Jieyou Xu (comment): looking for a reviewer familiar with attribute handling / HIR/MIR changes
    • The current challenge seems to be that existing attribute handling only supports a "single path segment", i.e. #[contracts(<inner_expr>)] but not #[contracts::requires(<inner_expr>)]. We also can't trivially reuse existing register_attr! or visitor because they impose strict limits on the <inner_expr> and require it to be very primitive (like literals or identifiers or simple lists) but does not support arbitrary expressions like x > 0. To support the arbitrary inner expression (in that it may not necessarily be a valid rust expression), it may require relaxing that restriction, but it may raise further parsing/grammar questions. It's not clear what's the best path forward here.

RFC

  • No I-compiler-nominated RFCs this time.

Oldest PRs waiting for review

T-compiler

  • "Implement unification of const abstract impls" rust#104803 (last review activity: 12 months ago)
    • this is assigned to Oli which is now on vacation (and removed themself from a number of pending reviews). If anyone wants to pick it up here …
  • "Do not eagerly reject inference vars when trying to resolve method calls." rust#126316 (last review activity: about 53 days ago)
    • cc @lcnr

Next week's WG checkins

None

Next meetings' agenda draft: hackmd link

Select a repo