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

Disable NEON intrinsics on big-endian ARM #1485

Merged
merged 1 commit into from
Oct 21, 2023

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Oct 19, 2023

These are currently broken because the order of elements inside vectors is reversed on big-endian systems: the ARM ABI requires that element 0 is located at the highest address of the vector type. However LLVM intrinsics expect element 0 to be located at the lowest address.

See https://llvm.org/docs/BigEndianNEON.html and arm_neon.h in Clang for more details.

This is tracked in #1484.

Although this is a breaking change, this is acceptable for 2 reasons:

  • big endian ARM targets are only tier 3.
  • it is preferable to stop existing code from compiling than to let it run and produce incorrect results.

@rustbot
Copy link
Collaborator

rustbot commented Oct 19, 2023

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

@Amanieu
Copy link
Member Author

Amanieu commented Oct 19, 2023

@rust-lang/libs-api @rust-lang/libs

This is a breaking change, but a necessary one in my opinion until the intrinsics are fixed. They currently silently produce the wrong results, which has led to a bug in hashbrown on big-endian ARM: rust-lang/rust#116880

These are currently broken because the order of elements inside
vectors is reversed on big-endian systems: the ARM ABI requires that
element 0 is located at the highest address of the vector type. However
LLVM intrinsics expect element 0 to be located at the lowest address.

See https://llvm.org/docs/BigEndianNEON.html and `arm_neon.h` in
Clang for more details.

Although this is a breaking change, this is acceptable for 2 reasons:
- big endian ARM targets are only tier 3.
- it is preferable to stop existing code from compiling than to let it
run and produce incorrect results.
@Amanieu
Copy link
Member Author

Amanieu commented Oct 19, 2023

@rustbot ping arm

@rustbot
Copy link
Collaborator

rustbot commented Oct 19, 2023

Error: The feature ping is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@Amanieu
Copy link
Member Author

Amanieu commented Oct 19, 2023

@jacobbramley
Copy link
Contributor

Fixing this is not likely to be quick or easy, so I agree that disabling them for BE targets is probably the best approach for now.

@Amanieu Amanieu merged commit 70760fa into rust-lang:master Oct 21, 2023
26 checks passed
@Amanieu Amanieu deleted the disable_be_neon branch October 21, 2023 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants