Skip to content

allow using shims for aarch64_be#5075

Open
folkertdev wants to merge 1 commit into
rust-lang:masterfrom
folkertdev:miri-aarch64_be
Open

allow using shims for aarch64_be#5075
folkertdev wants to merge 1 commit into
rust-lang:masterfrom
folkertdev:miri-aarch64_be

Conversation

@folkertdev
Copy link
Copy Markdown
Contributor

Related to #5065, it's fine to just allow shims on aarch64_be.

I've also removed some transmutes from the tests, they are very likely to cause endianness issues.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 28, 2026

Thank you for contributing to Miri! A reviewer will take a look at your PR, typically within a week or two.
Please remember to not force-push to the PR branch except when you need to rebase due to a conflict or when the reviewer asks you for it.

@rustbot rustbot added the S-waiting-on-review Status: Waiting for a review to complete label May 28, 2026
Comment on lines +855 to 857
// We currently only test on little-endian, but big-endian aarch64 does exist.
return shims::aarch64::EvalContextExt::emulate_aarch64_intrinsic(
this, link_name, abi, args, dest,
Copy link
Copy Markdown
Member

@RalfJung RalfJung May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... are the implementations correct on BE? Do we have any way to check that?

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... are the implementations correct on BE?

Unclear. I believe stdarch is kinda broken actually. Which is weird because we run the whole test suite etc but I think we've messed up loads and stores.

When I run the neon shim test file with qemu it falls over on the first assert. The test code translated to C works just fine, so stdarch is doing something wrong.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case I'd prefer to wait until we have confidence in our implementation, before enabling the intrinsics again. Or is that somehow making it harder to fix stdarch?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rustbot author
based on above comment

@rustbot rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels May 29, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 29, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: Waiting for the PR author to address review comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants