aarch64 fix for big endian types

[lucarlig]

NEON intrinsics are not supported on aarch64 big endian targets. Change to use fall back types for big endian aarch64.
tested on aarch64_be-unknown-linux-gnu_ilp32 and aarch64_be-unknown-linux-gnu

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.46%. Comparing base (a9f09d7) to head (fc4691e).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1786   +/-   ##
=======================================
  Coverage   87.46%   87.46%           
=======================================
  Files          16       16           
  Lines        6021     6021           
=======================================
  Hits         5266     5266           
  Misses        755      755           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[joshlf]

Thanks for this! A few small things:

• Can you rebase on main and force-push this as a single commit?
• Can you update your commit message to follow our commit message format?
• Is there a citation for the fact that NEON intrinsics aren't supported on big-endian targets? If you could add a comment with a citation in the source code, that'd be good.
• Can you add an include section here and use matrix includes to add the following two build combinations. Please add a comment that references this PR and explains why we test on these combinations.
  • toolchain: nightly / target: aarch64_be-unknown-linux-gnu_ilp32 / features: --all-features / crate: zerocopy
  • toolchain: nightly / target: aarch64_be-unknown-linux-gnu / features: --all-features / crate: zerocopy

[lucarlig]

@joshlf fixed the stuff that you requested,
this is the issue you are looking for as evidence rust-lang/stdarch#1484
I don't think the ilp32 version tests anything different so I skipped including that

[lucarlig]

it's failing because 'aarch64_be-unknown-linux-gnu' is not provided by rustup, only rustc
to test should do something like:

cargo build --target=aarch64_be-unknown-linux-gnu -Zbuild-std

[joshlf]

it's failing because 'aarch64_be-unknown-linux-gnu' is not provided by rustup, only rustc to test should do something like:

cargo build --target=aarch64_be-unknown-linux-gnu -Zbuild-std

Ah gotcha. I confirmed locally that this works:

$ ./cargo.sh +nightly build --target=aarch64_be-unknown-linux-gnu -Zbuild-std --features simd

Can you add a new build target (under kani and before check_fmt) that runs that command? That can replace the other changes to .github/workflows/ci.yml.

[joshlf]

Okay amazing, thank you for your work on this! Once this merges, I can publish a new release that includes it.

[joshlf]

Unfortunately I think that the build failure you're currently seeing is going to require you to compute the correct nightly to install based on the contents of Cargo.toml. You can extract that via ./cargo.sh --version nightly and then tell the GitHub action to install that particular nightly.

Here's how we do that in the build matrix - here it should be much simpler because you only need to support a single toolchain and none of the Miri stuff is relevant:

zerocopy/.github/workflows/ci.yml

Lines 264 to 321 in a9f09d7

	- name: Configure environment variables
	run: |
	set -eo pipefail
	# We use toolchain descriptors ("msrv", "stable", "nightly", and values
	# from the "metadata.build-rs" key in Cargo.toml) in the matrix. This
	# step converts the current descriptor to a particular toolchain version
	# by looking up the corresponding key in `Cargo.toml`. It sets the
	# `ZC_TOOLCHAIN` environment variable for use in the next step
	# (toolchain installation) because GitHub variable interpolation doesn't
	# support running arbitrary commands. In other words, we can't rewrite:
	#
	# toolchain: $ {{ env.ZC_TOOLCHAIN }}
	#
	# ...to:
	#
	# toolchain: $ {{ ./cargo.sh --version matrix.toolchain }} # hypothetical syntax
	ZC_TOOLCHAIN="$(./cargo.sh --version ${{ matrix.toolchain }})"
	echo "Found that the '${{ matrix.toolchain }}' toolchain is $ZC_TOOLCHAIN" | tee -a $GITHUB_STEP_SUMMARY
	echo "ZC_TOOLCHAIN=$ZC_TOOLCHAIN" >> $GITHUB_ENV
	if [[ '${{ matrix.toolchain }}' == 'nightly' ]]; then
	RUSTFLAGS="$RUSTFLAGS $ZC_NIGHTLY_RUSTFLAGS"
	MIRIFLAGS="$MIRIFLAGS $ZC_NIGHTLY_MIRIFLAGS"
	echo "Using nightly toolchain; setting RUSTFLAGS='$RUSTFLAGS' and MIRIFLAGS='$MIRIFLAGS'" | tee -a $GITHUB_STEP_SUMMARY
	echo "RUSTFLAGS=$RUSTFLAGS" >> $GITHUB_ENV
	echo "MIRIFLAGS=$MIRIFLAGS" >> $GITHUB_ENV
	else
	echo "Using non-nightly toolchain; not modifying RUSTFLAGS='$RUSTFLAGS' or MIRIFLAGS='$MIRIFLAGS'" | tee -a $GITHUB_STEP_SUMMARY
	fi
	# On our MSRV, `cargo` does not know about the `rust-version` field. As a
	# result, in `cargo.sh`, if we use our MSRV toolchain in order to run `cargo
	# metadata`, we will not be able to extract the `rust-version` field. Thus,
	# in `cargo.sh`, we explicitly do `cargo +stable metadata`. This requires a
	# (more recent) stable toolchain to be installed. As of this writing, this
	# toolchain is not used for anything else.
	- name: Install stable Rust for use in 'cargo.sh'
	uses: dtolnay/rust-toolchain@00b49be78f40fba4e87296b2ead62868750bdd83 # stable
	with:
	toolchain: stable
	- name: Install Rust with ${{ matrix.toolchain }} toolchain (${{ env.ZC_TOOLCHAIN }}) and target ${{ matrix.target }}
	uses: dtolnay/rust-toolchain@00b49be78f40fba4e87296b2ead62868750bdd83 # stable
	with:
	toolchain: ${{ env.ZC_TOOLCHAIN }}
	targets: ${{ matrix.target }}
	# We require the `rust-src` component to ensure that the compiler
	# error output generated during UI tests matches that generated on
	# local developer machines; see
	# https://github.com/rust-lang/rust/issues/116433.
	#
	# Only nightly has a working Miri, so we skip installing on all other
	# toolchains. This expression is effectively a ternary expression -
	# see [1] for details.
	#
	# [1] https://github.com/actions/runner/issues/409#issuecomment-752775072
	components: clippy, rust-src ${{ matrix.toolchain == 'nightly' && ', miri' || '' }}

[lucarlig]

@joshlf not sure what dependency is missing

[joshlf]

@joshlf not sure what dependency is missing

You need to edit this line:

zerocopy/.github/workflows/ci.yml

Line 707 in a9f09d7

needs: [build_test, kani, check_fmt, check_readme, check_versions, generate_cache, check-all-toolchains-tested, check-job-dependencies, run-git-hooks]

...to include check_be_aarch64

[joshlf]

Thanks again for your work on this!

[workingjubilee]

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)

[joshlf]

@workingjubilee is the lack of support in zerocopy breaking you? I'm hesitant for us to own a fix in stdarch unless there are users who actively need this feature.

[workingjubilee]

@joshlf Just to be clear, you're fine. If you want to accept this fix, that's okay. But I just don't think that fixing zerocopy, memchr, half, etc., in a fix that might be best-off reverted once stdarch is fixed, is better than fixing stdarch.

[joshlf]

@joshlf Just to be clear, you're fine. If you want to accept this fix, that's okay. But I just don't think that fixing zerocopy, memchr, half, etc., in a fix that might be best-off reverted once stdarch is fixed, is better than fixing stdarch.

Okay gotcha, that makes sense.