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

Fix types for ARM SIMD32 intrinsics #1679

Merged
merged 2 commits into from
Nov 26, 2024
Merged

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Nov 19, 2024

These were previously defined using vector types which is incorrect. Instead, {u}int{8x4,16x2}_t are aliases for i32 and u32.

This also fixes CI since these types don't need to be passed in NEON registers and this was triggering a newly added compiler warning.

@rustbot
Copy link
Collaborator

rustbot commented Nov 19, 2024

@Amanieu: no appropriate reviewer found, use r? to override

These were previously defined using vector types which is incorrect.
Instead, `{u}int{8x4,16x2}_t` are aliases for `i32` and `u32`.

This also fixes CI since these types don't need to be passed in NEON
registers and this was triggering a newly added compiler warning.
@Amanieu
Copy link
Member Author

Amanieu commented Nov 19, 2024

@heiher LoongArch seems to have been broken by rust-lang/rust#116558 as well, but I think the lint may be triggering incorrectly in that case? Perhaps this needs to be fixed on the compiler side?

@Amanieu
Copy link
Member Author

Amanieu commented Nov 19, 2024

Sorry that should be @heiher.

@heiher
Copy link
Contributor

heiher commented Nov 20, 2024

@heiher LoongArch seems to have been broken by rust-lang/rust#116558 as well, but I think the lint may be triggering incorrectly in that case? Perhaps this needs to be fixed on the compiler side?

Thank you for pointing it out. I have created a PR for it: rust-lang/rust#133249

@Amanieu Amanieu enabled auto-merge (rebase) November 26, 2024 22:51
@Amanieu
Copy link
Member Author

Amanieu commented Nov 26, 2024

cc @alexcrichton I've disabled WASM in CI because it was failing for the same reason as loongarch: the compiler is emitting an error because it thinks that SIMD types can't be passed as argument/return values when simd128 is not enabled and the vector constructor functions don't have this attribute. However in practice it's fine since when that target feature is absent LLVM will just lower to scalar operations.

@Amanieu Amanieu merged commit cfd48f9 into rust-lang:master Nov 26, 2024
54 checks passed
@alexcrichton
Copy link
Member

Thanks for the heads up! I'll take a look early next week into what the best fix would be

@alexcrichton
Copy link
Member

Followed up with #1686

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants