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

target_arch is wrong in arm64ec-pc-windows-msvc #131172

Open
madsmtm opened this issue Oct 2, 2024 · 6 comments
Open

target_arch is wrong in arm64ec-pc-windows-msvc #131172

madsmtm opened this issue Oct 2, 2024 · 6 comments
Labels
C-bug Category: This is a bug. O-AArch64 Armv8-A or later processors in AArch64 mode O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@madsmtm
Copy link
Contributor

madsmtm commented Oct 2, 2024

Rust's target_arch cfg is generally very generic, and does not include e.g. the sub-architecture.

For example, the arm64e-apple-darwin has target_arch = "aarch64". This is also true of all the ARM targets, these all have target_arch = "arm" even though the full target name is much longer.

arm64ec-pc-windows-msvc is the only target that differs here, and sets target_arch = "arm64ec". I am not familiar with the specifics of ARM64EC, but I think it should probably set target_arch = "aarch64" too? Or, if it really needs to be different, target_arch = "aarch64ec", since Rust prefers Aarch64 over ARM64? Or maybe target_arch = "aarch64", but with an extra target_feature to allow distinguishing them?

I would've filed a PR to fix it myself, but it's a bit of work since a lot of places in rustc check for target.arch == "arm64ec", so I wanted to make sure I was on the right path first.

CC target maintainer @dpaoliello.
@rustbot label O-windows O-AArch64

@madsmtm madsmtm added the C-bug Category: This is a bug. label Oct 2, 2024
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. O-AArch64 Armv8-A or later processors in AArch64 mode O-windows Operating system: Windows labels Oct 2, 2024
@dpaoliello
Copy link
Contributor

There was also a good thread discussing this here: rust-lang/stdarch#1550 (comment)

As for arm64ec versus aarch64ec, every other tool and doc uses arm64ec so I'd rather be consistent with that. There's also precedent for ARM64 - the arm64e-apple-darwin architecture that you mention.

@workingjubilee
Copy link
Member

tbh, I still would have preferred we had went with this as a target_env or target_abi, precisely because I think that is a weak precedent. but I'm not interested in dying on that hill, and I agree that arm64ec is a very weird kitbash of x86 and aarch64, so in an important sense we can't rely on intrinsics/assembly working normally, so using target_arch for this is like "eh, sure".

@madsmtm
Copy link
Contributor Author

madsmtm commented Oct 2, 2024

@madsmtm See the platform support documentation: https://github.com/rust-lang/rust/blob/44722bd9ba50426191f78d12138a2ae1a12affe7/src/doc/rustc/src/platform-support/arm64ec-pc-windows-msvc.md#reusing-code-from-other-architectures---x86_64-or-aarch64

Does it answer your questions?

Ah, I was sorta just batch-opening issues, didn't research that closely enough.

There was also a good thread discussing this here: rust-lang/stdarch#1550 (comment)

Thanks for the link!

As for arm64ec versus aarch64ec, every other tool and doc uses arm64ec so I'd rather be consistent with that. There's also precedent for ARM64 - the arm64e-apple-darwin architecture that you mention.

Hmm, I seemed to remember discussion somewhere that arm64e-apple-darwin being badly named, but what I could find in #73628 (comment) seems to suggest that arm64e is a fair name because of the proprietary-ness?

@workingjubilee
Copy link
Member

workingjubilee commented Oct 2, 2024

relevantly, target_arch and the """target triple""" don't actually always match.

@madsmtm
Copy link
Contributor Author

madsmtm commented Oct 2, 2024

Personally, I don't really have a stake in this, I'll leave the issue open though for T-compiler (or is target_arch T-lang?) to discuss and decide (since it does not seem like that happened on the PR introducing the target, and the linked discussion in stdarch both didn't reach agreement and happened outside this repo).

@rustbot label T-compiler

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 2, 2024
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-AArch64 Armv8-A or later processors in AArch64 mode O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants