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

Update cpuinfo.h: add support for Windows ARM64EC #227

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

snnn
Copy link
Contributor

@snnn snnn commented Mar 4, 2024

To fix a build error:

src\arm\windows\init-by-logical-sys-info.c(309,17): Error C2065: 'cpuinfo_uarchs': undeclared identifier

In short, Arm64EC is a new application binary interface (ABI) for apps running on Arm devices with an x64 like ABI.

Reference:

  1. VC++ predefined-macros
  2. ARM64EC overview

@snnn snnn changed the title Update cpuinfo.h: add support for ARM64EC Update cpuinfo.h: add support for Windows ARM64EC Mar 4, 2024
@@ -18,15 +18,15 @@
#define CPUINFO_ARCH_X86 1
#endif

#if defined(__x86_64__) || defined(__x86_64) || defined(_M_X64) || defined(_M_AMD64)
#if defined(__x86_64__) || defined(__x86_64) || (defined(_M_X64) && !defined(_M_ARM64EC)) || (defined(_M_AMD64) && !defined(_M_ARM64EC))
Copy link

Choose a reason for hiding this comment

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

I checked https://learn.microsoft.com/en-us/cpp/preprocessor/predefined-macros?view=msvc-170
MSVC architecture related defination is complex, suggest to add more comments.

Copy link
Contributor

@malfet malfet left a comment

Choose a reason for hiding this comment

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

It feels wrong to define CPUINFO_ARCH_X86_64 on ARM platform, please explain why this is necessary for Win ARM64

@@ -18,15 +18,15 @@
#define CPUINFO_ARCH_X86 1
#endif

#if defined(__x86_64__) || defined(__x86_64) || defined(_M_X64) || defined(_M_AMD64)
#if defined(__x86_64__) || defined(__x86_64) || (defined(_M_X64) && !defined(_M_ARM64EC)) || (defined(_M_AMD64) && !defined(_M_ARM64EC))
Copy link
Contributor

Choose a reason for hiding this comment

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

Defining CPUINFO_ARCH_X86_64 for _M_ARM64EC feels wrong, please explain why this is needed

Copy link
Contributor Author

@snnn snnn May 9, 2024

Choose a reason for hiding this comment

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

_M_X64 Defined as the integer literal value 100 for compilations that target x64 processors or ARM64EC. So _M_X64 could be defined when the CPU is actually an ARM CPU.
Here the condition is defined(_M_X64) && !defined(_M_ARM64EC)) . When _M_ARM64EC is 1, the expression is false.

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.

4 participants