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

Array-as-Immediate array ABI is missing noundef parameter metadata #123183

Open
scottmcm opened this issue Mar 29, 2024 · 0 comments
Open

Array-as-Immediate array ABI is missing noundef parameter metadata #123183

scottmcm opened this issue Mar 29, 2024 · 0 comments
Labels
A-codegen Area: Code generation A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@scottmcm
Copy link
Member

scottmcm commented Mar 29, 2024

Today, these

#[no_mangle]
pub fn replace_short_array_1(r: &mut [u32; 1], v: [u32; 1]) -> [u32; 1] {}

#[no_mangle]
pub fn replace_short_array_2(r: &mut [u32; 2], v: [u32; 2]) -> [u32; 2] {}

give https://play.rust-lang.org/?version=nightly&mode=release&edition=2021&gist=084e57540370e8a6a2c51dd98053396a

define i32 @replace_short_array_1(ptr noalias nocapture noundef align 4 dereferenceable(4) %r, i32 %0) unnamed_addr #0 { … }

define i64 @replace_short_array_2(ptr noalias nocapture noundef align 4 dereferenceable(8) %r, i64 %0) unnamed_addr #0 { … }

Note specifically how they have i32 %0 and i64 %0 -- that means we're not telling LLVM that the array must be initialized (non-undef and non-poison).

#106294 did this for normal scalars. For example if you change that first one to

#[no_mangle]
pub fn replace_short_array_1_alt(r: &mut [u32; 1], v: u32) -> [u32; 1] {
    std::mem::replace(r, [v])
}

then you correctly get i32 noundef %v.

It looks like it was just missed in some of the trickier ABI cases.

@scottmcm scottmcm added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-codegen Area: Code generation labels Mar 29, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 29, 2024
@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 3, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Apr 3, 2024
…<try>

Add `noundef` metadata for fits-in-target-pointer-size array immediate arguments

`noundef` is only added if the small array immediate fits in the target pointer size and if optimizations are enabled.

Closes rust-lang#123183.
bors added a commit to rust-lang-ci/rust that referenced this issue Apr 16, 2024
…davidtwco,DianQK

Add `noundef` metadata for fits-in-target-pointer-size array immediate arguments

`noundef` is only added if the small array immediate fits in the target pointer size and if optimizations are enabled.

Closes rust-lang#123183.
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 1, 2024
…<try>

Annotate eligible small immediate arguments with `noundef`

Retake of rust-lang#123425.

We try to annotate small (fits within target pointer width) aggregate arguments passed as immediates
(specifically casted as "appropriately sized integer type") with `noundef`.

Example:

```rs
#[no_mangle]
pub fn short_array_u64x1(v: [u64; 1]) -> [u64; 1] { v }
```

currently produces

```llvm
define i64 `@short_array_u64x1(i64` %0) ...
```

This PR changes that to

```llvm
define noundef i64 `@short_array_u64x1(i64` noundef %0) ...
```

The `noundef` attribute is added only if the immediate value has no padding. Specifically,
the conservative heuristic we use is to:

- Peel away layers of `#[repr(Rust)]` or `#[repr(transparent)]` wrappers if present
- Check for innermost simple arrays (whose element type is a primitive type) that can fit within
  target pointer width

Union immediates or otherwise anything that contains unions will not have `noundef` attribute
applied.

Closes rust-lang#123183.

cc `@/RalfJung` who pointed out various problems with the previous take, hopefully I addressed most of
them in this take.

r? `@ghost` (perf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
3 participants