Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 11 pull requests #137001

Merged
merged 27 commits into from
Feb 14, 2025
Merged

Rollup of 11 pull requests #137001

merged 27 commits into from
Feb 14, 2025

Conversation

workingjubilee
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

chenyukang and others added 27 commits February 12, 2025 09:56
Previously, we unconditionally set the bitwidth to 128-bits, the largest
an discrimnator would possibly be. Then, LLVM would cut down the constant by
chopping off leading zeroes before emitting the DWARF. LLVM only
supported 64-bit descriminators, so this would also have occasionally
resulted in truncated data (or an assert) if more than 64-bits were
used.

LLVM added support for 128-bit enumerators in llvm/llvm-project#125578

That patchset also trusts the constant to describe how wide the variant tag is.
As a result, we went from emitting tags that looked like:
DW_AT_discr_value     (0xfe)

(`form1`)

to emitting tags that looked like:
DW_AT_discr_value	(<0x10> fe ff ff ff 00 00 00 00 00 00 00 00 00 00 00 00 )

This makes the `DW_AT_discr_value` encode at the bitwidth of the tag,
which:
1. Is probably closer to our intentions in terms of describing the data.
2. Doesn't invoke the 128-bit support which may not be supported by all
   debuggers / downstream tools.
3. Will result in smaller debug information.
(S)ccache can be useful for more things that just LLVM. For example, we will soon want to use it also for GCC, and theoretically also for building stage0 Rust tools.
This continues two ongoing projects:

- Replacing ascii art with real icons that don't look like
  syntax, are understandable to people who're familiar with
  desktop computers and smart devices, and aren't ugly.
- Using labels and tooltips to clarify these icons, when the
  limits of popular iconography hit us. In this case, I've added
  tooltips, because, unfortunately, there's not room for
  always-visible labels.
When preparing a function's coverage counters and metadata during codegen, any
part of the original coverage graph that was removed by MIR optimizations can
be treated as having an execution count of zero.

Somewhat counter-intuitively, if we give those unreachable nodes a _higher_
priority for receiving physical counters (instead of counter expressions), that
ends up reducing the total number of physical counters needed.

This works because if a node is unreachable, we don't actually create a
physical counter for it. Instead that node gets a fixed zero counter, and any
other node that would have relied on that physical counter in its counter
expression can just ignore that term completely.
I told rhelmot to do this in rust-lang#134913. But it's not correct; compiletest
shouldn't inherit RUSTFLAGS at all.

Pass a single new --host-rustcflags to compiletest instead, without overwriting any
existing arguments.

Fixes the following failure, which only happens when building llvm from
source and then running `x test --stage 1 ui-fulldeps`:
```
diff --git a/tests/ui-fulldeps/fluent-messages/test.stderr b/tests/ui-fulldeps/fluent-messages/test.stderr
index 0b3bb14ce51..978ac46c5a2 100644
--- a/tests/ui-fulldeps/fluent-messages/test.stderr
+++ b/tests/ui-fulldeps/fluent-messages/test.stderr
@@ -1,3 +1,8 @@
+warning[E0602]: unknown lint: `linker_messages`
+   |
+   = note: requested on the command line with `-A linker_messages`
+   = note: `#[warn(unknown_lints)]` on by default
```
Signed-off-by: onur-ozkan <work@onurozkan.dev>
"stage 1" for fulldeps means "compile with stage 0, link against stage 1".
But this code wanted to switch on the compiler that's building, not the
compiler that's being tested. Fix the check.

Previously, it would fail with a warning about linker-messages:
```
--- stderr -------------------------------
warning[E0602]: unknown lint: `linker_messages`
   |
   = note: requested on the command line with `-A linker_messages`
   = note: `#[warn(unknown_lints)]` on by default
```
…rors

rework rigid alias handling

Necessary for rust-lang#136824 if we treat coinductive cycles as errors as we otherwise don't emit an error for

```rust
trait Overflow {
    type Assoc;
}
impl<T> Overflow for T {
    type Assoc = <T as Overflow>::Assoc;
}
```

The important part is that we only add a `RigidAlias` candidate in cases where the alias is actually supposed to be rigid:
- its trait bound has been proven via a `ParamEnv` or `ItemBound` candidate
- it's one of the special builtin traits which have a blanket impl with a `default` assoc type

This means that we now more explicitly control which aliases should rigid to avoid accidentally accepting cyclic aliases. This requires changes to diagnostics as we no longer enter an explicit `RigidAlias` candidate for `NormalizesTo` goals whose trait bound doesn't hold.

To fix this I've modified the `BestObligation` visitor always ignore `RigidAlias` candidates and to instead manually check these requirements if there are no applicable candidates. I also removed the hack for handling `structurally_normalize_ty` failures. This fixes rust-lang#134905 as we no longer continue to use the `EvalCtxt` even though a nested goal failed.

r? ``@compiler-errors``
…inding, r=estebank

Fix diagnostic when using = instead of : in let binding

Fixes rust-lang#133713

r? ``@estebank``
debuginfo: Set bitwidth appropriately in enum variant tags

Previously, we unconditionally set the bitwidth to 128-bits, the largest an enum would possibly be. Then, LLVM would cut down the constant by chopping off leading zeroes before emitting the DWARF. LLVM only supported 64-bit enumerators, so this would also have occasionally resulted in truncated data.

LLVM added support for 128-bit enumerators in llvm/llvm-project#125578

That patchset trusts the constant to describe how wide the variant tag is, so the high 64-bits of zeros are considered potentially load-bearing.

As a result, we went from emitting tags that looked like:
DW_AT_discr_value     (0xfe)

(because `dwarf::BestForm` selected `data1`)

to emitting tags that looked like:
DW_AT_discr_value	(<0x10> fe ff ff ff 00 00 00 00 00 00 00 00 00 00 00 00 )

This makes the `DW_AT_discr_value` encode at the bitwidth of the tag, which:
1. Is probably closer to our intentions in terms of describing the data.
2. Doesn't invoke the 128-bit support which may not be supported by all debuggers / downstream tools.
3. Will result in smaller debug information.
…piler-errors

eagerly prove WF when resolving fully qualified paths

fixes rust-lang/trait-system-refactor-initiative#161.

This hopefully shouldn't impact perf. I do think we need to deal with at least part of the fallout here, opening for vibes.

r? ``@compiler-errors``
Move `llvm.ccache` to `build.ccache`

(S)ccache can be useful for more things that just LLVM. For example, we will soon want to use it also for GCC, and theoretically also for building stage0 Rust tools (rust-lang#136921, https://rust-lang.zulipchat.com/#narrow/channel/242791-t-infra/topic/Using.20sccache.20for.20Rust).

r? ``@onur-ozkan``
…ttons, r=GuillaumeGomez

rustdoc: use better, consistent SVG icons for scraped examples

## Screenshots

![](https://github.com/user-attachments/assets/f305fb20-5ded-428a-b0d0-04e8b7762769)

![](https://github.com/user-attachments/assets/5b9bee5e-74b9-447b-a19a-49f32b6bf218)

![image](https://github.com/user-attachments/assets/d855a8c8-dc24-44f9-a067-1e0f0654c28a)

![image](https://github.com/user-attachments/assets/71bca54a-0562-480a-8989-938acc351307)

## Description

This continues two ongoing projects

- Replacing ascii art with real icons that don't look like syntax, are understandable to people who're familiar with desktop computers and smart devices, and aren't ugly.
- Using labels and tooltips to clarify these icons, when the limits of popular iconography hit us. In this case, I've added tooltips, because, unfortunately, there's not room for always-visible labels.

r? ``@GuillaumeGomez``
coverage: Eliminate more counters by giving them to unreachable nodes

When preparing a function's coverage counters and metadata during codegen, any part of the original coverage graph that was removed by MIR optimizations can be treated as having an execution count of zero.

Somewhat counter-intuitively, if we give those unreachable nodes a _higher_ priority for receiving physical counters (instead of counter expressions), that ends up reducing the total number of physical counters needed.

This works because if a node is unreachable, we don't actually create a physical counter for it. Instead that node gets a fixed zero counter, and any other node that would have relied on that physical counter in its counter expression can just ignore that term completely.
Compiletest should not inherit all host RUSTFLAGS

I told ``@rhelmot`` to do this in rust-lang#134913. But it's not correct; compiletest shouldn't inherit RUSTFLAGS at all.

Pass a single new --host-rustcflags to compiletest instead, without overwriting any existing arguments.

Fixes the following failure, which only happens when building llvm from source and then running `x test --stage 1 ui-fulldeps`:
```
diff --git a/tests/ui-fulldeps/fluent-messages/test.stderr b/tests/ui-fulldeps/fluent-messages/test.stderr
index 0b3bb14ce51..978ac46c5a2 100644
--- a/tests/ui-fulldeps/fluent-messages/test.stderr
+++ b/tests/ui-fulldeps/fluent-messages/test.stderr
``@@`` -1,3 +1,8 ``@@``
+warning[E0602]: unknown lint: `linker_messages`
+   |
+   = note: requested on the command line with `-A linker_messages`
+   = note: `#[warn(unknown_lints)]` on by default
```

See https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/.E2.9C.94.20unknown.20lint.3A.20.60linker_messages.60.20when.20blessing.20tests.20on.20.2E.2E.2E for more context.
…ouxu

unify LLVM version finding logic

kind a self-explanatory
ci: move `x86_64-gnu-debug` job to the free runner

try-job: x86_64-gnu-debug
Fix `x test --stage 1 ui-fulldeps` on macOS (until the next beta bump)

"stage 1" for fulldeps means "compile with stage 0, link against stage 1". But this code wanted to switch on the compiler that's building, not the compiler that's being tested. Fix the check.

Previously, it would fail with a warning about linker-messages:
```
--- stderr -------------------------------
warning[E0602]: unknown lint: `linker_messages`
   |
   = note: requested on the command line with `-A linker_messages`
   = note: `#[warn(unknown_lints)]` on by default
```

cc https://rust-lang.zulipchat.com/#narrow/channel/182449-t-compiler.2Fhelp/topic/unknown.20lint.3A.20.60linker_messages.60.20when.20blessing.20tests.20on.20.2E.2E.2E, rust-lang#136960
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Feb 14, 2025
@rustbot rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) rollup A PR which is a rollup labels Feb 14, 2025
@workingjubilee
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Feb 14, 2025

📌 Commit 6eb3929 has been approved by workingjubilee

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 14, 2025
@bors
Copy link
Contributor

bors commented Feb 14, 2025

⌛ Testing commit 6eb3929 with merge 6dfeab5...

@bors
Copy link
Contributor

bors commented Feb 14, 2025

☀️ Test successful - checks-actions
Approved by: workingjubilee
Pushing 6dfeab5 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 14, 2025
@bors bors merged commit 6dfeab5 into rust-lang:master Feb 14, 2025
7 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 14, 2025
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#136863 rework rigid alias handling dc6c67c27698a6ec1c03bf945be86bfaad7f3a49 (link)
#136869 Fix diagnostic when using = instead of : in let binding 653bfceedb85bb63b8a877a89cb29f34c135bf6f (link)
#136895 debuginfo: Set bitwidth appropriately in enum variant tags 928ce35cb589ad8460e899c46effac487fb8ed6f (link)
#136928 eagerly prove WF when resolving fully qualified paths 4b77d8ef12a7f77e01c46b95d3d067855caa42a1 (link)
#136941 Move llvm.ccache to build.ccache 6e660f53f4b6b8e7e91d5651b584cab055634321 (link)
#136950 rustdoc: use better, consistent SVG icons for scraped examp… 28442a5886c8fcbed1306198d564ff8ecf6b41e0 (link)
#136957 coverage: Eliminate more counters by giving them to unreach… fa914c0d58976ca7b8efee10fdfddfeb29d499db (link)
#136960 Compiletest should not inherit all host RUSTFLAGS 42103408460b37aad83bd522057c1d0dd3755327 (link)
#136962 unify LLVM version finding logic d4eb9d1caed80ece385ce4de2505fa8817b2481c (link)
#136970 ci: move x86_64-gnu-debug job to the free runner 19b4adddf31f82c9ec555ea63a9daccb60acc5a3 (link)
#136973 Fix x test --stage 1 ui-fulldeps on macOS (until the next… 261079feb3282c2ff66b325389afcde6593f7acc (link)

previous master: a567209daa

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6dfeab5): comparison URL.

Overall result: ❌ regressions - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
0.8% [0.8%, 0.9%] 2
Regressions ❌
(secondary)
0.1% [0.1%, 0.1%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.8% [0.8%, 0.9%] 2

Max RSS (memory usage)

Results (primary 0.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
2.3% [2.3%, 2.3%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.2% [-2.2%, -2.2%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.1% [-2.2%, 2.3%] 2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 789.427s -> 789.679s (0.03%)
Artifact size: 347.75 MiB -> 347.73 MiB (-0.01%)

@Kobzol
Copy link
Contributor

Kobzol commented Feb 19, 2025

The small regressions were caused by #136928, but it was a fix, so no further action needed.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Feb 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.