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 12 pull requests #136954

Merged
merged 26 commits into from
Feb 13, 2025
Merged

Rollup of 12 pull requests #136954

merged 26 commits into from
Feb 13, 2025

Conversation

jhpratt
Copy link
Member

@jhpratt jhpratt commented Feb 13, 2025

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

calebzulawski and others added 26 commits January 27, 2025 23:44
It is speculated that these two can be conceptually merged, and it can
start by ripping out rustc's notion of the PtxKernel call convention.
Leave the ExternAbi for now, but the nvptx target now should see it as
just a different way to spell Conv::GpuKernel.
Pointers for variables all need to be in the same address space for
correct compilation. Therefore ensure that even if an `alloca` is
created in a different address space, it is casted to the default
address space before its value is used.

This is necessary for the amdgpu target and others where the default
address space for `alloca`s is not 0.

For example the following code compiles incorrectly when not casting the
address space to the default one:

```rust
fn f(p: *const i8 /* addrspace(0) */) -> *const i8 /* addrspace(0) */ {
    let local = 0i8; /* addrspace(5) */
    let res = if cond { p } else { &raw const local };
    res
}
```

results in

```llvm
    %local = alloca addrspace(5) i8
    %res = alloca addrspace(5) ptr

if:
    ; Store 64-bit flat pointer
    store ptr %p, ptr addrspace(5) %res

else:
    ; Store 32-bit scratch pointer
    store ptr addrspace(5) %local, ptr addrspace(5) %res

ret:
    ; Load and return 64-bit flat pointer
    %res.load = load ptr, ptr addrspace(5) %res
    ret ptr %res.load
```

For amdgpu, `addrspace(0)` are 64-bit pointers, `addrspace(5)` are
32-bit pointers.
The above code may store a 32-bit pointer and read it back as a 64-bit
pointer, which is obviously wrong and cannot work. Instead, we need to
`addrspacecast %local to ptr addrspace(0)`, then we store and load the
correct type.
…e conversion functions

Having these implementation available crate-wide means that platforms not using sockets for their networking code have to stub out the libc definitions required to support them. This PR moves the conversions to private helper functions that are only available where actually needed.

I also fixed the signature of the function converting from a C socket address to a Rust one: taking a reference to a `sockaddr_storage` resulted in unsound usage inside  `LookupHost::next`, which could create a reference to a structure smaller than `sockaddr_storage`. Thus I've replaced the argument type with a pointer and made the function `unsafe`.
This will be used in Clippy to detect unbuffered calls to
`Read::bytes()`.
Stabilize target_feature_11

# Stabilization report

This is an updated version of rust-lang#116114, which is itself a redo of rust-lang#99767. Most of this commit and report were copied from those PRs. Thanks ```@LeSeulArtichaut``` and ```@calebzulawski!```

## Summary
Allows for safe functions to be marked with `#[target_feature]` attributes.

Functions marked with `#[target_feature]` are generally considered as unsafe functions: they are unsafe to call, cannot *generally* be assigned to safe function pointers, and don't implement the `Fn*` traits.

However, calling them from other `#[target_feature]` functions with a superset of features is safe.

```rust
// Demonstration function
#[target_feature(enable = "avx2")]
fn avx2() {}

fn foo() {
    // Calling `avx2` here is unsafe, as we must ensure
    // that AVX is available first.
    unsafe {
        avx2();
    }
}

#[target_feature(enable = "avx2")]
fn bar() {
    // Calling `avx2` here is safe.
    avx2();
}
```

Moreover, once rust-lang#135504 is merged, they can be converted to safe function pointers in a context in which calling them is safe:

```rust
// Demonstration function
#[target_feature(enable = "avx2")]
fn avx2() {}

fn foo() -> fn() {
    // Converting `avx2` to fn() is a compilation error here.
    avx2
}

#[target_feature(enable = "avx2")]
fn bar() -> fn() {
    // `avx2` coerces to fn() here
    avx2
}
```

See the section "Closures" below for justification of this behaviour.

## Test cases
Tests for this feature can be found in [`tests/ui/target_feature/`](https://github.com/rust-lang/rust/tree/f6cb952dc115fd1311b02b694933e31d8dc8b002/tests/ui/target-feature).

## Edge cases
### Closures
 * [target-feature 1.1: should closures inherit target-feature annotations? rust-lang#73631](rust-lang#73631)

Closures defined inside functions marked with #[target_feature] inherit the target features of their parent function. They can still be assigned to safe function pointers and implement the appropriate `Fn*` traits.

```rust
#[target_feature(enable = "avx2")]
fn qux() {
    let my_closure = || avx2(); // this call to `avx2` is safe
    let f: fn() = my_closure;
}
```
This means that in order to call a function with #[target_feature], you must guarantee that the target-feature is available while the function, any closures defined inside it, as well as any safe function pointers obtained from target-feature functions inside it, execute.

This is usually ensured because target features are assumed to never disappear, and:
- on any unsafe call to a `#[target_feature]` function, presence of the target feature is guaranteed by the programmer through the safety requirements of the unsafe call.
- on any safe call, this is guaranteed recursively by the caller.

If you work in an environment where target features can be disabled, it is your responsibility to ensure that no code inside a target feature function (including inside a closure) runs after this (until the feature is enabled again).

**Note:** this has an effect on existing code, as nowadays closures do not inherit features from the enclosing function, and thus this strengthens a safety requirement. It was originally proposed in rust-lang#73631 to solve this by adding a new type of UB: “taking a target feature away from your process after having run code that uses that target feature is UB” .
This was motivated by userspace code already assuming in a few places that CPU features never disappear from a program during execution (see i.e. https://github.com/rust-lang/stdarch/blob/2e29bdf90832931ea499755bb4ad7a6b0809295a/crates/std_detect/src/detect/arch/x86.rs); however, concerns were raised in the context of the Linux kernel; thus, we propose to relax that requirement to "causing the set of usable features to be reduced is unsafe; when doing so, the programmer is required to ensure that no closures or safe fn pointers that use removed features are still in scope".

* [Fix #[inline(always)] on closures with target feature 1.1 rust-lang#111836](rust-lang#111836)

Closures accept `#[inline(always)]`, even within functions marked with `#[target_feature]`. Since these attributes conflict, `#[inline(always)]` wins out to maintain compatibility.

### ABI concerns
* [The extern "C" ABI of SIMD vector types depends on target features rust-lang#116558](rust-lang#116558)

The ABI of some types can change when compiling a function with different target features. This could have introduced unsoundness with target_feature_11, but recent fixes (rust-lang#133102, rust-lang#132173) either make those situations invalid or make the ABI no longer dependent on features. Thus, those issues should no longer occur.

### Special functions
The `#[target_feature]` attribute is forbidden from a variety of special functions, such as main, current and future lang items (e.g. `#[start]`, `#[panic_handler]`), safe default trait implementations and safe trait methods.

This was not disallowed at the time of the first stabilization PR for target_features_11, and resulted in the following issues/PRs:
* [`#[target_feature]` is allowed on `main` rust-lang#108645](rust-lang#108645)
* [`#[target_feature]` is allowed on default implementations rust-lang#108646](rust-lang#108646)
* [#[target_feature] is allowed on #[panic_handler] with target_feature 1.1 rust-lang#109411](rust-lang#109411)
* [Prevent using `#[target_feature]` on lang item functions rust-lang#115910](rust-lang#115910)

## Documentation
 * Reference: [Document the `target_feature_11` feature reference#1181](rust-lang/reference#1181)
---

cc tracking issue rust-lang#69098
cc ```@workingjubilee```
cc ```@RalfJung```
r? ```@rust-lang/lang```
Cast allocas to default address space

Pointers for variables all need to be in the same address space for correct compilation. Therefore ensure that even if an `alloca` is created in a different address space, it is casted to the default address space before its value is used.

This is necessary for the amdgpu target and others where the default address space for `alloca`s is not 0.

For example the following code compiles incorrectly when not casting the address space to the default one:

```rust
fn f(p: *const i8 /* addrspace(0) */) -> *const i8 /* addrspace(0) */ {
    let local = 0i8; /* addrspace(5) */
    let res = if cond { p } else { &raw const local };
    res
}
```

results in

```llvm
    %local = alloca addrspace(5) i8
    %res = alloca addrspace(5) ptr

if:
    ; Store 64-bit flat pointer
    store ptr %p, ptr addrspace(5) %res

else:
    ; Store 32-bit scratch pointer
    store ptr addrspace(5) %local, ptr addrspace(5) %res

ret:
    ; Load and return 64-bit flat pointer
    %res.load = load ptr, ptr addrspace(5) %res
    ret ptr %res.load
```

For amdgpu, `addrspace(0)` are 64-bit pointers, `addrspace(5)` are 32-bit pointers.
The above code may store a 32-bit pointer and read it back as a 64-bit pointer, which is obviously wrong and cannot work. Instead, we need to `addrspacecast %local to ptr addrspace(0)`, then we store and load the correct type.

Tracking issue: rust-lang#135024
…ler-errors

Reject `?Trait` bounds in various places where we unconditionally warned since 1.0

fixes rust-lang#135730
fixes rust-lang#135809

Also a breaking change, so let's see what crater says.

This has been an unconditional warning since *before* 1.0
Mark condition/carry bit as clobbered in C-SKY inline assembly

C-SKY's compare and some arithmetic/logical instructions modify condition/carry bit (C) in PSR, but there is currently no way to mark it as clobbered in `asm!`.

This PR marks it as clobbered except when [`options(preserves_flags)`](https://doc.rust-lang.org/reference/inline-assembly.html#r-asm.options.supported-options.preserves_flags) is used.

Refs:
- Section 1.3 "Programming model" and Section 1.3.5 "Condition/carry bit" in CSKY Architecture user_guide:
  https://github.com/c-sky/csky-doc/blob/9f7121f7d40970ba5cc0f15716da033db2bb9d07/CSKY%20Architecture%20user_guide.pdf

  > Under user mode, condition/carry bit (C) is located in the lowest bit of PSR, and it can be
accessed and changed by common user instructions. It is the only data bit that can be visited
under user mode in PSR.

  > Condition or carry bit represents the result after one operation. Condition/carry bit can be
clearly set according to the results of compare instructions or unclearly set as some
high-precision arithmetic or logical instructions. In addition, special instructions such as
DEC[GT,LT,NE] and XTRB[0-3] will influence the value of condition/carry bit.

- Register definition in LLVM:
  https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/llvm/lib/Target/CSKY/CSKYRegisterInfo.td#L88

cc ```@Dirreke``` ([target maintainer](https://github.com/rust-lang/rust/blob/aa6f5ab18e67cb815f73e0d53d217bc54b0da924/src/doc/rustc/src/platform-support/csky-unknown-linux-gnuabiv2.md#target-maintainers))

r? ```@Amanieu```

```@rustbot``` label +O-csky +A-inline-assembly
std: replace the `FromInner` implementation for addresses with private conversion functions

Having these implementation available crate-wide means that platforms not using sockets for their networking code have to stub out the libc definitions required to support them. This PR moves the conversions to private helper functions that are only available where actually needed.

I also fixed the signature of the function converting from a C socket address to a Rust one: taking a reference to a `sockaddr_storage` resulted in unsound usage inside  `LookupHost::next`, which could create a reference to a structure smaller than `sockaddr_storage`. Thus I've replaced the argument type with a pointer and made the function `unsafe`.
…pitit, r=compiler-errors

Fix cycle when debug-printing opaque types from RPITIT

Extend rust-lang#66594 to opaque types from RPITIT.

Before this PR, enabling debug logging like `RUSTC_LOG="[check_type_bounds]"` for code containing RPITIT produces a query cycle of `explicit_item_bounds`, as pretty printing for opaque type calls [it](https://github.com/rust-lang/rust/blob/d9a4a47b8b3dc0bdff83360cea2013200d60d49c/compiler/rustc_middle/src/ty/print/pretty.rs#L1001).
…he-arcradeongeforce, r=bjorn3

compiler: internally merge `PtxKernel` into `GpuKernel`

r? ``@bjorn3`` for review
…-escape, r=notriddle

Correctly escape hashtags when running `invalid_rust_codeblocks` lint

Fixes rust-lang#136899.

We forgot to use `map_line` when we wrote this lint.

r? ``@notriddle``
Update books

## rust-lang/book

1 commits in e2fa4316c5a7c0d2499c5d6b799adcfad6ef7a45..d4d2c18cbd20876b2130a546e790446a8444cb32
2025-02-12 16:22:54 UTC to 2025-02-12 16:22:54 UTC

- Fix example for `cargo fix` (rust-lang/book#4226)

## rust-lang/reference

3 commits in de2d5289e45506b11dd652bef4f99de64be70e1c..6195dbd70fc6f0980c314b4d23875ac570d8253a
2025-02-12 15:40:30 UTC to 2025-02-11 20:35:17 UTC

- Revert "Update function-pointer.md for stabilization of `extended_varargs_abi_support`" (rust-lang/reference#1734)
- Rework error handling in mdbook-spec (rust-lang/reference#1733)
- `as` cast improvements (rust-lang/reference#1732)
…=fmease

Add diagnostic item for `std::io::BufRead`

This will be used in Clippy to detect unbuffered calls to `Read::bytes()`.
… r=nnethercote

Reinstate nnethercote in the review rotation.

I'm back from vacation.

r? ``@nnethercote``
@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-meta Area: Issues & PRs about the rust-lang/rust repository itself O-solid Operating System: SOLID S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 13, 2025
@rustbot rustbot added T-compiler Relevant to the compiler 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Feb 13, 2025
@jhpratt
Copy link
Member Author

jhpratt commented Feb 13, 2025

@bors r+ rollup=never p=5

@bors
Copy link
Contributor

bors commented Feb 13, 2025

📌 Commit d16b067 has been approved by jhpratt

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 13, 2025
@bors
Copy link
Contributor

bors commented Feb 13, 2025

⌛ Testing commit d16b067 with merge 9fcc9cf...

@bors
Copy link
Contributor

bors commented Feb 13, 2025

☀️ Test successful - checks-actions
Approved by: jhpratt
Pushing 9fcc9cf to master...

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

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#134090 Stabilize target_feature_11 8c75afd3efee8e9d18fb6eb6500e0e730dc45431 (link)
#135025 Cast allocas to default address space 26079a3d461ea190e0ac778c1c3d96b20164063b (link)
#135841 Reject ?Trait bounds in various places where we unconditi… c6883cfb7170b36d88968c2d499fb2c86b08026c (link)
#136217 Mark condition/carry bit as clobbered in C-SKY inline assem… f8c17acbebf93dd42fda3b2e9518c7d69fd08bdc (link)
#136699 std: replace the FromInner implementation for addresses w… bc1eafc40e1d100247e2fb16c3d4c60a0e67a2e5 (link)
#136806 Fix cycle when debug-printing opaque types from RPITIT c8da70a1e96c525ba26fab6503e66a82475e7f17 (link)
#136807 compiler: internally merge PtxKernel into GpuKernel a4ecaebbbdbdf7f53d1c031b50f9f1e7bc99ce8c (link)
#136818 Implement read*_exact for std:io::repeat b034e8b7aef5539d8db04f048f809d1633597a03 (link)
#136927 Correctly escape hashtags when running `invalid_rust_codebl… acfe3f145a7031c4e65ec3ab2ccb42b48b21e280 (link)
#136937 Update books 0315682736a2bbde3532306e93d3f587a9ed29b7 (link)
#136945 Add diagnostic item for std::io::BufRead 7939fe815e51dfc8a215d0df873858e246791743 (link)
#136947 Reinstate nnethercote in the review rotation. ea8e4ff1de9331a45bc49be342a0196d82fce64c (link)

previous master: 6dce9f8c2d

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 (9fcc9cf): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

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
Regressions ❌
(secondary)
0.3% [0.2%, 0.4%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -1.8%, secondary -2.7%)

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)
1.5% [1.5%, 1.5%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-5.1% [-5.1%, -5.1%] 1
Improvements ✅
(secondary)
-2.7% [-3.1%, -2.3%] 4
All ❌✅ (primary) -1.8% [-5.1%, 1.5%] 2

Cycles

Results (secondary -2.3%)

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)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.3% [-2.6%, -2.1%] 2
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 788.472s -> 788.14s (-0.04%)
Artifact size: 347.85 MiB -> 347.85 MiB (-0.00%)

@jhpratt jhpratt deleted the rollup-koefsot branch February 14, 2025 03:35
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Feb 15, 2025
Rollup of 12 pull requests

Successful merges:

 - rust-lang#134090 (Stabilize target_feature_11)
 - rust-lang#135025 (Cast allocas to default address space)
 - rust-lang#135841 (Reject `?Trait` bounds in various places where we unconditionally warned since 1.0)
 - rust-lang#136217 (Mark condition/carry bit as clobbered in C-SKY inline assembly)
 - rust-lang#136699 (std: replace the `FromInner` implementation for addresses with private conversion functions)
 - rust-lang#136806 (Fix cycle when debug-printing opaque types from RPITIT)
 - rust-lang#136807 (compiler: internally merge `PtxKernel` into `GpuKernel`)
 - rust-lang#136818 (Implement `read*_exact` for `std:io::repeat`)
 - rust-lang#136927 (Correctly escape hashtags when running `invalid_rust_codeblocks` lint)
 - rust-lang#136937 (Update books)
 - rust-lang#136945 (Add diagnostic item for `std::io::BufRead`)
 - rust-lang#136947 (Reinstate nnethercote in the review rotation.)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-meta Area: Issues & PRs about the rust-lang/rust repository itself merged-by-bors This PR was explicitly merged by bors. O-solid Operating System: SOLID 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-compiler Relevant to the compiler 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.