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 9 pull requests #136930

Closed
wants to merge 25 commits into from

Conversation

GuillaumeGomez
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

taiki-e and others added 25 commits January 29, 2025 06:46
We choose to test for Linux and Windows instead of random other targets.
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.
This is needed for stdarch which now includes s390x target features.
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.
This should guarantee it tests what we want it to test and no more.
It should probably also run on 64-bit platforms that are not x86-64,
which will often have the vector registers the opt implies.
We should remove entire `cc2ar` but `cc` doesn't seem to cover all the conditions that `cc2ar` handles.
For now, I replaced the `else` logic only, which is a bit hacky and unstable.

Signed-off-by: onur-ozkan <[email protected]>
…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`.
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
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`.
…or-tests, r=saethlin

tests: `-Copt-level=3` instead of `-O` in assembly tests

An effective blocker for redefining the meaning of `-O` is to stop reusing this somewhat ambiguous alias in our own assembly test suite. The choice between `-Copt-level=2` and `-Copt-level=3` is arbitrary for most of our tests. In most cases it makes no difference, so I set most of them to `-Copt-level=3`, as it will lead to slightly more "normalized" assembly.
…or-codegen-tests, r=saethlin

tests: `-Copt-level=3` instead of `-O` in codegen tests

An effective blocker for redefining the meaning of `-O` is to stop reusing this somewhat ambiguous alias in our own codegen test suite. The choice between `-Copt-level=2` and `-Copt-level=3` is arbitrary for most of our tests. In most cases it makes no difference, so I set most of them to `-Copt-level=3`, as it will lead to slightly more "normalized" codegen.

try-job: test-various
try-job: arm-android
try-job: armhf-gnu
try-job: i686-gnu-1
try-job: i686-gnu-2
try-job: i686-mingw
try-job: i686-msvc-1
try-job: i686-msvc-2
try-job: aarch64-apple
try-job: aarch64-gnu
…he-arcradeongeforce, r=bjorn3

compiler: internally merge `PtxKernel` into `GpuKernel`

r? `@bjorn3` for review
Update stdarch

Updates stdarch

41 commits in 684de0d6fef708cae08214fef9643dd9ec7296e1..818c71dae1c612bd7b4ffba69e02c8d7270a18f6
2024-12-21 12:02:28 +0000 to 2025-02-09 12:57:14 -0800

- Update all stdarch crates to Rust 2024: rust-lang/stdarch#1710
- Fix some test naming, and refactor stdarch-verify in general: rust-lang/stdarch#1707
- Fix build and CLI behaviour for stdarch-gen-arm. rust-lang/stdarch#1705
- s390x: add vec_sub, vec_mul, vec_min, vec_max, vec_abs and vec_splats: rust-lang/stdarch#1704
- add vec_add for s390x: rust-lang/stdarch#1703
- add is_s390x_feature_detected: rust-lang/stdarch#1699
- Fix typo and prettify comment: rust-lang/stdarch#1697
- Tidying x86 `as_*` functions: rust-lang/stdarch#1696
- Expand feature detection on AArch64 Darwin: rust-lang/stdarch#1695
- Fix the bug in CMPINT intrinsics with IMM3=7: rust-lang/stdarch#1694
- New ARM intrinsic generator: rust-lang/stdarch#1693
- core_arch: Add LoongArch basic intrinsics: rust-lang/stdarch#1688
use cc archiver as default in `cc2ar`

We should remove entire `cc2ar` but `cc` doesn't seem to cover all the conditions that `cc2ar` handles. For now, I replaced the `else` logic only, which is a bit hacky and unstable.

Fixes rust-lang#136759
@rustbot rustbot added O-solid Operating System: SOLID 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-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 12, 2025
@rustbot rustbot added the rollup A PR which is a rollup label Feb 12, 2025
@GuillaumeGomez
Copy link
Member Author

@bors r+ p=5 rollup=never

@bors
Copy link
Contributor

bors commented Feb 12, 2025

📌 Commit 9fe4c6a has been approved by GuillaumeGomez

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 12, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 12, 2025
…llaumeGomez

Rollup of 9 pull requests

Successful merges:

 - rust-lang#135025 (Cast allocas to default address space)
 - 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#136758 (tests: `-Copt-level=3` instead of `-O` in assembly tests)
 - rust-lang#136761 (tests: `-Copt-level=3` instead of `-O` in codegen tests)
 - rust-lang#136807 (compiler: internally merge `PtxKernel` into `GpuKernel`)
 - rust-lang#136818 (Implement `read*_exact` for `std:io::repeat`)
 - rust-lang#136831 (Update stdarch)
 - rust-lang#136916 (use cc archiver as default in `cc2ar`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Feb 12, 2025

⌛ Testing commit 9fe4c6a with merge 1398103...

@rust-log-analyzer
Copy link
Collaborator

The job aarch64-apple failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::assert_failed_inner
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
"/Users/runner/work/rust/rust/build/aarch64-apple-darwin/stage1-tools/cg_clif/build/example/neon" exited with status ExitStatus(unix_wait_status(6))
Command has failed. Rerun with -v to see more details.
  local time: Wed Feb 12 19:13:03 UTC 2025
  network time: Wed, 12 Feb 2025 19:13:03 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@bors
Copy link
Contributor

bors commented Feb 12, 2025

💔 Test failed - checks-actions

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

I commented on the 3 PRs I think are the potential ones which failed in the CI.

@GuillaumeGomez GuillaumeGomez deleted the rollup-hjsijmf branch February 12, 2025 19:19
@bjorn3
Copy link
Member

bjorn3 commented Feb 12, 2025

It could have been this miscompilation: rust-lang/rustc_codegen_cranelift#1560

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-solid Operating System: SOLID rollup A PR which is a rollup 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-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.