chore: added Apple M1 platform detection when compiling with Rosetta 2#40315
chore: added Apple M1 platform detection when compiling with Rosetta 2#40315kyr0 wants to merge 1 commit intonodejs:mainfrom
Conversation
configure.py
Outdated
| k = cc_macros(os.environ.get('CC_host')) | ||
|
|
||
| matchup = { | ||
| '_LP64' : 'arm64', |
There was a problem hiding this comment.
_LP64 is not arm64 specific so this is going to break non-arm 64-bit platforms.
There was a problem hiding this comment.
True, I just came to the same understanding while you were commenting. I wrote a sum-up of the issue in my updated PR desc.
There was a problem hiding this comment.
The true cause of the issue is that when we assume that the parsed output of e.g. ${CC_host} -dM -E - will contain the host arch at a specific index like we do here:
Line 1072 in d23af0d
Just isn't there because the output of cc (in my case) is not standardized (as it seems) or other circumstances make the toolchain write stuff like _LP64 there instead of the host arch identifier... well, then we're going to assume the wrong host_arch here, trigger the cross-compilation, and v8 compilation will fail because it tries to compile the wrong inline assembly impl.
Any ideas? :)
There was a problem hiding this comment.
I could just put a special override if-condition to check for M1 as a platform instead of relying on the cc output but it is such a nice abstract impl, that it seems a bit hacky to do that. However, Node.js not being able to compile on M1 is quite a blocker... so maybe...
There was a problem hiding this comment.
Unfortunately, in the case of building with a fat binary, even running uname -p or uname -m gives me i386 and x86_64. This is probably because Rosetta2 emulates "I'm running on an x86_64 machine". There is only one way to escape this rabbit hole (as far as I can see atm): Running uname -v which gives us the kernel version string as Darwin Kernel Version 20.6.0: Mon Aug 30 06:12:20 PDT 2021; root:xnu-7195.141.6~3/RELEASE_ARM64_T8101.
This off behaviour can not reproduced by running uname -p or uname -m on the command line of an M1 machine. It can only reproduced when the parent process that executes these shell commands is emulated via Rosetta.
As we can see, the uname -v kernel version string includes the ARM64 release tag which indicates that the kernel is actually running on an ARM platform and the toolchain is running as a universal binary, translated via Rosetta2.
d23af0d to
b2c1f89
Compare
|
I now implemented this extra check using the |
| stdin=subprocess.PIPE, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE) | ||
| except OSError: |
There was a problem hiding this comment.
Could someone check if other types of error/exception could happen in other environments when uname is not available? I tried with different command names that are def. not available and the catch/forget code works well
b2c1f89 to
3ea3a9b
Compare
|
Instead of relying on uname, wouldn't a better solution to check the value of the sysctl Apparently you can also query the macOS I/O registry to get the real underlying hardware, such as with However should we really be supporting this? I don't own/use Apple hardware so I'm not familiar with Rosetta, but it seems like with a change like this we'd be implicitly cross compiling (with no way to create an x86 binary), which might be surprising? |
|
@mscdex Nice suggestions! Actually, it is a valid point that you raised; the default build target could also be x86_64 for Rosetta; but it would be nice (from my point of view) that if you're running Rosetta, it wouldn't crash at least, when you specify |
AshCripps
left a comment
There was a problem hiding this comment.
Blocking as I believe this will break our release builds and will need verifying.
This PR handles the special case when a developer is trying to compile on an Apple Silicon M1 machine with a toolchain that is dual-arch (
arm64eandx86_64) which is a valid case when the build toolchain isx86_64based and Rosetta 2 is active. In this case, the currentconfigure.pyis unable to figure out the correct host platform as the toolchain will just returnx86_64even if the true host arch isarm64.The impl. in the commit is trying to keep the current logic as untouched as possible to prevent any regressions.
Fixes: #40302