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

Make this crate build on big-endian aarch64. #162

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

he32
Copy link

@he32 he32 commented Sep 29, 2024

As noted in rust-lang/stdarch#1484, the NEON intrinsics are broken on big-endian aarch64.

This is part of fixing rust to build for & on big-endian aarch64, following up rust-lang/rust#129819.

As noted in rust-lang/stdarch#1484,
the NEON intrinsics are broken on big-endian aarch64.

This is part of fixing rust to build for & on big-endian aarch64,
following up rust-lang/rust#129819.
@BurntSushi
Copy link
Owner

Can you add a big endian test target to CI?

@he32
Copy link
Author

he32 commented Sep 29, 2024

Can you add a big endian test target to CI?

Sorry, I have no idea how I do that.
I do have a local qemu-run big-endian NetBSD/aarch64 10.0 installation running. However, it appears that for this to work, it depends on which version of qemu is used -- I'm currently using qemu 8.1.3, and there it works, but it does not look like qemu 8.0.2 works. I do not think I've done testing for qemu 9.0.1. Then there's the issue of how to match this up with CI, which I've never done before.

@BurntSushi
Copy link
Owner

Have you looked into using cross? CI is already using cross for many targets (which uses qemu under the hood). Ideally you would find a big endian target and just add it to CI.

@he32
Copy link
Author

he32 commented Sep 29, 2024 via email

@BurntSushi
Copy link
Owner

I don't have a ton of time for mentoring, but if you're looking to fix big endian targets, then we should add testing for those targets to CI. Otherwise there is nothing ensuring that your fixes are correct or that the problems stay fixed.

Please look at the existing CI configuration in .github. You should see cross being used.

@BurntSushi
Copy link
Owner

See also https://github.com/cross-rs/cross

Cross has nothing to do with "the cloud."

@he32
Copy link
Author

he32 commented Oct 2, 2024

Hmm... This crate is vendored into the rust compiler. WIthout these fixes, there will not be a big-endian aarch64 rust compiler without having to manually apply patches, and without the rust compiler, I predict there will be problems setting up the CI. So how do we break this logjam?

@BurntSushi
Copy link
Owner

What have you tried? What happened? Did you investigate cross?

@he32
Copy link
Author

he32 commented Oct 2, 2024 via email

@BurntSushi
Copy link
Owner

Fair enough. This means I'm going to need to investigate myself and I don't know when I'm going to have time to do that.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 2, 2024
This is done by avoiding attempts at using neon / SIMD in
big-endian mode by patching some of the vendored crates.
Neon / SIMD is known to be problematical in rust, ref.
rust-lang/stdarch#1484, even
though the CPU itself supports it.

I've also tried reporting the memchr fixes upstream, ref.
BurntSushi/memchr#162
So far not yet adopted.

Zerocopy has also received a pull request:
google/zerocopy#1795
@he32
Copy link
Author

he32 commented Oct 2, 2024

For what it's worth, I present to you the build result of your crate with my fixes applied,
run on a big-endian aarch64 host, ref. the attachment.
typescript.txt

@lucarlig
Copy link

lucarlig commented Oct 3, 2024

for the CI, the setup from google/zerocopy#1786 might help? and the issue from the rust compiler: rust-lang/stdarch#1484
not sure if any of this can help.

@workingjubilee
Copy link

Please fix this by fixing the intrinsics upstream. Fixing this in every single crate is not less effort than fixing it in stdarch: rust-lang/stdarch#1484 (comment)

taiki-e added a commit to taiki-e/atomic-maybe-uninit that referenced this pull request Nov 8, 2024
rust-lang/rust#132714 bumped memchr without
considering aarch64_be is not supported in memchr:
BurntSushi/memchr#162
taiki-e added a commit to taiki-e/portable-atomic that referenced this pull request Nov 9, 2024
rust-lang/rust#132714 bumped memchr without
considering aarch64_be is not supported in memchr:
BurntSushi/memchr#162
taiki-e added a commit to taiki-e/setup-cross-toolchain-action that referenced this pull request Nov 10, 2024
rust-lang/rust#132714 bumped memchr without
considering aarch64_be is not supported in memchr:
BurntSushi/memchr#162
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