Skip to content

Conversation

sayantn
Copy link
Contributor

@sayantn sayantn commented Sep 11, 2025

Fixes: #1904

This is the correct behavior according to the Intrinsics Guide, which specifies that these should compile to NOP. Also, GCC and Clang matches this behavior.

I am a bit surprised that this didn't get caught in the AArch64_BE intrinsic_test run!

@rustbot
Copy link
Collaborator

rustbot commented Sep 11, 2025

r? @folkertdev

rustbot has assigned @folkertdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@folkertdev
Copy link
Contributor

cc @Jamesbarford @adamgemmell

@adamgemmell
Copy link
Contributor

adamgemmell commented Sep 12, 2025

I'm also really confused why the intrinsic-test tool doesn't catch this. Printing out the values you can see that the intrinsic does indeed get tested and it matches C. At the same time this PR clearly changes their behaviour but still passes the test tool

@folkertdev
Copy link
Contributor

I've confirmed locally that the rust intrinsic-test-programs binary emits the same output on master and on this branch, but also that the generated assembly is in fact different (as expected based on the source code). So, eh, weird?

Just confirming with the Arm folks: the implementation in this PR is the correct one, right?

@Jamesbarford
Copy link
Contributor

I've confirmed locally that the rust intrinsic-test-programs binary emits the same output on master and on this branch, but also that the generated assembly is in fact different (as expected based on the source code). So, eh, weird?

Just confirming with the Arm folks: the implementation in this PR is the correct one, right?

Looking through how it is implemented in arm_neon.h, there is no shuffling. This, to me, looks to be correct

@folkertdev folkertdev added this pull request to the merge queue Sep 15, 2025
Merged via the queue into rust-lang:master with commit 38863ea Sep 15, 2025
63 checks passed
@sayantn sayantn deleted the fix-vreinterpret branch October 8, 2025 10:32
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.

vreinterpret on big endian swaps byte order

5 participants