Cast allocas to default address space by Flakebi · Pull Request #135025 · rust-lang/rust

@rustbot rustbot added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

labels

Jan 2, 2025

Snektron

nikic

nikic

@Flakebi

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.

@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 11, 2025

fmease added a commit to fmease/rust that referenced this pull request

Feb 11, 2025
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

bors added a commit to rust-lang-ci/rust that referenced this pull request

Feb 11, 2025
Rollup of 7 pull requests

Successful merges:

 - rust-lang#134090 (Stabilize target_feature_11)
 - rust-lang#134999 (Add cygwin target.)
 - rust-lang#135025 (Cast allocas to default address space)
 - rust-lang#135408 (x86: make SSE2 required for i686 hardfloat targets and use it to pass SIMD types)
 - rust-lang#135549 (Document some safety constraints and use more safe wrappers)
 - rust-lang#136193 (Implement pattern type ffi checks)
 - rust-lang#136699 (std: replace the `FromInner` implementation for addresses with private conversion functions)

Failed merges:

 - rust-lang#136758 (tests: `-Copt-level=3` instead of `-O` in assembly tests)

r? `@ghost`
`@rustbot` modify labels: rollup

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request

Feb 12, 2025
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

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

jhpratt added a commit to jhpratt/rust that referenced this pull request

Feb 13, 2025
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

This was referenced

Feb 13, 2025

bors added a commit to rust-lang-ci/rust that referenced this pull request

Feb 13, 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

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request

Feb 13, 2025
Rollup merge of rust-lang#135025 - Flakebi:alloca-addrspace, r=nikic

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

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