Skip to content

Conversation

matthiaskrgr
Copy link
Member

@matthiaskrgr matthiaskrgr commented Aug 31, 2025

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

197g and others added 16 commits August 9, 2025 18:12
There are a few alternatives to the implementation. The principal
problem is that the selected value must be owned (in the sense of having
any drop flag of sorts) when the unselected value is dropped, such that
panic unwind goes through the drop of both. This ownership must then be
passed on in return when the drop went smoothly. The basic way of
achieving this is by extracting the selected value first, at the cost of
relying on the optimizer a little more for detecting the copy as
constructing the return value despite having a place in the body.
It seems important for LLVM that we select on the values by-value
instead of reading and have no intermediate store. So make sure the
guards selects both potential drops but defers the return value to the
second selection. Since the two options alias we use raw mutable
pointers instead of mutable references as before.
To unblock the Rust CI in PR [1], use a temporary commit from Rust for
Linux that supports the future target spec format.

Link: rust-lang#144443 [1]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
The current documentation is not clear whether adding `a` to a pointer overaligns (align up) or underaligns (align down).

It should say this explicitly.
Instead of a tuple, select the dropped value and its guard with two
separate calls to the intrinsic which makes both calls have a
pointer-valued argument that should be simpler in codegen. Use the same
condition on all (not an inverted condition) to clarify the intent of
parallel selection. This should also be a simpler value-dependency chain
if the guard is deduced unused (i.e. drop_in_place a noop for the type).
…r-width, r=Noratrieb

Make target pointer width in target json an integer

r? Noratrieb
cc `@RalfJung` (https://github.com/rust-lang/rust/pull/142352/files#r2230380120)

try-job: x86_64-rust-for-linux
…able-drop, r=joboet

Ensure consistent drop for panicking drop in hint::select_unpredictable

There are a few alternatives to the implementation. The principal problem is that the selected value must be owned (in the sense of having a drop flag of sorts) when the unselected value is dropped, such that panic unwind goes through the drop of both. This ownership must then be passed on in return when the drop went smoothly.

The basic way of achieving this is by extracting the selected value first, at the cost of relying on the optimizer a little more for detecting the copy as constructing the return value despite having a place in the body. Unfortunately, that causes LLVM to discard the !unpredictable annotation (for some reason that is beyond my comprehension of LLVM).

<details>
<summary>Extract from the build log showing an unannotated select being used</summary>

```
2025-08-09T16:51:06.8790764Z            39: define noundef i64 `@test_int2(i1` noundef zeroext %p, i64 noundef %a, i64 noundef %b) unnamed_addr #0 personality ptr `@rust_eh_personality` {
2025-08-09T16:51:06.8791368Z check:47'0                                  X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
2025-08-09T16:51:06.8791700Z            40: start:
2025-08-09T16:51:06.8791858Z check:47'0     ~~~~~~~
2025-08-09T16:51:06.8792043Z            41:  %ret.i = select i1 %p, i64 %a, i64 %b
2025-08-09T16:51:06.8792293Z check:47'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2025-08-09T16:51:06.8792686Z check:47'1               ?                             possible intended match
2025-08-09T16:51:06.8792946Z            42:  ret i64 %ret.i
2025-08-09T16:51:06.8793127Z check:47'0     ~~~~~~~~~~~~~~~~
```

</details>

So instead, this PR includes a guard to drop the selected `MaybeUnit<T>` which is active only for the section where the unselected value is dropped. That leaves the code for selecting the result intact leading to the expected ir. That complicates the 'unselection' process a little bit since we require _both_ values as a result of that intrinsic call. Since the arguments alias, this portion as well as the drop guard uses raw pointers.

Closes: rust-lang#145148
Prior: rust-lang#139977
…r=lcnr

Fix format string grammar in docs and improve alignment error message for rust-lang#144023

This PR improves error messages and documentation for format strings involving alignment and formatting traits.

Highlights:

- Clearer error messages for invalid alignment specifiers (e.g., `{0:#X>18}`), showing the expected `<`, `^`, or `>` and a working example:

    println!("{0:>#18X}", value);

- Updated UI test `format-alignment-hash.rs` to reflect the improved error output.
- Documentation clarification: ensures examples correctly show how width, alignment, and traits like `x`, `X`, `#` combine.

Motivation:
Previously, using `#` with alignment and width produced confusing errors. This PR guides users on the correct syntax and provides actionable examples.

Testing:
- Built the compiler (`./x build`)
- Blessed and ran UI tests (`./x. test src/test/ui/fmt/format-alignment-hash.rs --bless`)
- Verified full test suite passes (`./x test`)

Issue: rust-lang#144023
Clarify that align_offset overaligns

The current documentation is not clear whether adding `a` to a pointer overaligns (align up) or underaligns (align down).

It should say this explicitly.

cc `@nagisa`
@rustbot rustbot added A-CI Area: Our Github Actions CI A-compiler-builtins Area: compiler-builtins (https://github.com/rust-lang/compiler-builtins) A-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs 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) 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-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Aug 31, 2025
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=5

@bors
Copy link
Collaborator

bors commented Aug 31, 2025

📌 Commit e3881cb has been approved by matthiaskrgr

It is now in the queue for this repository.

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

bors commented Aug 31, 2025

⌛ Testing commit e3881cb with merge 564ee21...

@bors
Copy link
Collaborator

bors commented Aug 31, 2025

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 564ee21 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 31, 2025
@bors bors merged commit 564ee21 into rust-lang:master Aug 31, 2025
11 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 31, 2025
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#144443 Make target pointer width in target json an integer 45166c0e0d6029a8dada565f6c937a49fab26a13 (link)
#145174 Ensure consistent drop for panicking drop in hint::select_u… 0e7119fb2a499cfc846bbd8b6eca38c3f0c88420 (link)
#145592 Fix format string grammar in docs and improve alignment err… 504ae720dc08fb133a8a3a09a6c526d642da4f2f (link)
#145931 Clarify that align_offset overaligns beeaff1de69ac1c177579843753538e0d8c994eb (link)

previous master: 1bc901e0ca

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

Copy link
Contributor

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 1bc901e (parent) -> 564ee21 (this PR)

Test differences

Show 26 test diffs

Stage 1

  • hint::select_unpredictable_drop_on_panic: [missing] -> pass (J0)

Additionally, 25 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 564ee219127b796d56f74767366fd359758b97de --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. x86_64-rust-for-linux: 2488.8s -> 3212.9s (29.1%)
  2. x86_64-gnu-llvm-19: 2386.3s -> 2767.5s (16.0%)
  3. i686-gnu-2: 5430.8s -> 6228.5s (14.7%)
  4. dist-android: 1526.2s -> 1738.0s (13.9%)
  5. dist-apple-various: 3536.3s -> 4008.5s (13.4%)
  6. aarch64-gnu-llvm-19-1: 3333.2s -> 3720.4s (11.6%)
  7. test-various: 4591.2s -> 5124.2s (11.6%)
  8. i686-gnu-nopt-1: 7515.5s -> 8383.5s (11.5%)
  9. aarch64-gnu-debug: 4515.7s -> 4999.9s (10.7%)
  10. arm-android: 5773.9s -> 6392.0s (10.7%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (564ee21): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.2% [0.2%, 0.2%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -4.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
3.0% [3.0%, 3.0%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-8.1% [-11.5%, -4.7%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -4.4% [-11.5%, 3.0%] 3

Cycles

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

Binary size

Results (primary 0.2%, secondary 1.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
0.2% [0.1%, 0.3%] 22
Regressions ❌
(secondary)
1.2% [1.1%, 1.2%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.2% [0.1%, 0.3%] 22

Bootstrap: 468.036s -> 468.632s (0.13%)
Artifact size: 388.54 MiB -> 388.55 MiB (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CI Area: Our Github Actions CI A-compiler-builtins Area: compiler-builtins (https://github.com/rust-lang/compiler-builtins) A-compiletest Area: The compiletest test runner A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. 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-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants