-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix Linux ARM64 build #7051
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
Fix Linux ARM64 build #7051
Changes from 1 commit
8a0b400
830ee4b
6bba7ee
2b633a3
1cf0797
5dcba40
0c37dae
b2240be
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,14 @@ | |
| va_list _vl; \ | ||
| va_start(_vl, callInfo); \ | ||
| Js::Var* va = (Js::Var*)_vl | ||
| #elif defined(_ARM64_) && defined(__linux__) | ||
| // AAPCS64 (Linux ARM64 ABI) reference: | ||
| // https://github.com/ARM-software/abi-aa/blob/main/aapcs64/aapcs64.rst#appendix-variable-argument-lists | ||
| #define DECLARE_ARGS_VARARRAY(va, ...) \ | ||
| va_list _vl; \ | ||
| va_start(_vl, callInfo); \ | ||
| Js::Var* va = (Js::Var*)_vl.__stack + 2; \ | ||
| Assert(*reinterpret_cast<Js::CallInfo*>(va - 1) == callInfo) | ||
| #else | ||
| // We use a custom calling convention to invoke JavascriptMethod based on | ||
| // System ABI. At entry of JavascriptMethod the stack layout is: | ||
|
|
@@ -84,8 +92,19 @@ inline int _count_args(const T1&, const T2&, const T3&, const T4&, const T5&, Js | |
| #define CALL_ENTRYPOINT_NOASSERT(entryPoint, function, callInfo, ...) \ | ||
| entryPoint(function, callInfo, ##__VA_ARGS__) | ||
| #elif defined (_ARM64_) | ||
| #ifdef __linux__ | ||
| // Linux ARM64 uses AAPCS64: first 8 args in x0-x7, rest via stack. | ||
| // Fill x2-x7 with nulls here to force the expected stack layout: | ||
| // [RetAddr] [function] [callInfo] [args...] | ||
| #define CALL_ENTRYPOINT_NOASSERT(entryPoint, function, callInfo, ...) \ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd expect this to break the JavascriptStackWalker, I saw you said it was "fragile", what's the status?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The single hard-coded constant ArgOffsetFromFramePtr that JavascriptStackWalker relies on feels rather fragile to me. For now, I set ArgOffsetFromFramePtr=4 - I see 8 extra bytes of what seems like padding or saved registers. But I'd expect it to break with different compilers and optimization flags by analogy with my experience with DECLARE_ARGS_VARARRAY - at first (before I found va_list.__stack) I was trying to fixing it by trying different offsets in _get_va(), but it just kept breaking between optimized and debug builds. JavascriptStackWalker in JIT-less builds so far works for me with clang 19, 20, and 22, both optimized and debug builds. Perhaps because it only needs to deal with a few specific call sites in this mode. When JIT is enabled it is crashing, but because JIT wasn't supported on macOS, I didn't investigate these crashes further.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The JIT crash is not remarkable, the ARM JIT contains significant windows specific logic that needs re-writing for Linux/macOS including the way function entry and exit is setup and the managing of the stack layout. For calling JS functions in the interpreter we go via arm64_CallFunction to set the stack exactly the way we need for the stack walker and argument handling to work: https://github.com/chakra-core/ChakraCore/blob/master/lib/Runtime/Library/arm64/arm64_CallFunction.S |
||
| entryPoint(function, callInfo, nullptr, nullptr, nullptr, nullptr, nullptr, nullptr, \ | ||
| function, callInfo, ##__VA_ARGS__) | ||
| #else | ||
| // macOS has own bespoke vararg cc (DarwinPCS), varargs always passed via stack. | ||
| // Duplicate function/callInfo so they are pushed onto stack as part of varargs. | ||
| #define CALL_ENTRYPOINT_NOASSERT(entryPoint, function, callInfo, ...) \ | ||
| entryPoint(function, callInfo, function, callInfo, ##__VA_ARGS__) | ||
| #endif | ||
| #else | ||
| #error CALL_ENTRYPOINT_NOASSERT not yet implemented | ||
| #endif | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -122,12 +122,21 @@ typedef int __ptrace_request; | |
| ASSIGN_REG(R11) \ | ||
| ASSIGN_REG(R12) | ||
| #elif defined(_ARM64_) | ||
| #ifdef __linux__ | ||
| #define ASSIGN_CONTROL_REGS \ | ||
| ASSIGN_REG(PState) \ | ||
| ASSIGN_REG(Fp) \ | ||
| ASSIGN_REG(Sp) \ | ||
| ASSIGN_REG(Lr) \ | ||
| ASSIGN_REG(Pc) | ||
| #else | ||
| #define ASSIGN_CONTROL_REGS \ | ||
| ASSIGN_REG(Cpsr) \ | ||
| ASSIGN_REG(Fp) \ | ||
| ASSIGN_REG(Sp) \ | ||
| ASSIGN_REG(Lr) \ | ||
| ASSIGN_REG(Pc) | ||
| #endif | ||
|
|
||
| #define ASSIGN_INTEGER_REGS \ | ||
| ASSIGN_REG(X0) \ | ||
|
|
@@ -545,6 +554,45 @@ CONTEXT_SetThreadContext( | |
| return ret; | ||
| } | ||
|
|
||
| #if defined(__linux__) && defined(_ARM64_) | ||
| // Reference: https://github.com/dotnet/runtime/blob/main/src/coreclr/pal/src/thread/context.cpp#L927 | ||
| static inline fpsimd_context* _GetNativeSigSimdContext(unsigned char* data, size_t size) | ||
| { | ||
| size_t pos = 0; | ||
| while (pos < size) | ||
| { | ||
| _aarch64_ctx* ctx = reinterpret_cast<_aarch64_ctx*>(&data[pos]); | ||
| if (pos + sizeof(_aarch64_ctx) > size) | ||
| { | ||
| break; | ||
| } | ||
|
Comment on lines
+559
to
+568
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had another look at this, and I was wondering why it's so different to the function it's based on in dotnet, does the dotnet version require something we don't have or did it contain a performance trap?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I rewrote it from scratch. Dotnet's version is more general returning fpsimd_context and sve_context, but Chakra only needs fpsimd_context. |
||
| if (ctx->magic == FPSIMD_MAGIC) | ||
| { | ||
| return reinterpret_cast<fpsimd_context*>(ctx); | ||
| } | ||
| if (ctx->magic == EXTRA_MAGIC) | ||
| { | ||
| extra_context* extra = reinterpret_cast<extra_context*>(ctx); | ||
| fpsimd_context* fp = _GetNativeSigSimdContext(reinterpret_cast<unsigned char*>(extra->datap), extra->size); | ||
| if (fp) return fp; | ||
| } | ||
| if (ctx->size == 0) { | ||
| break; | ||
| } | ||
| pos += ctx->size; | ||
| } | ||
| return nullptr; | ||
| } | ||
|
|
||
| static inline fpsimd_context* GetNativeSigSimdContext(native_context_t* native) { | ||
| return _GetNativeSigSimdContext(static_cast<unsigned char*>(native->uc_mcontext.__reserved), sizeof(native->uc_mcontext.__reserved)); | ||
| } | ||
|
|
||
| static inline const fpsimd_context* GetConstNativeSigSimdContext(const native_context_t* native) { | ||
| return GetNativeSigSimdContext(const_cast<native_context_t*>(native)); | ||
| } | ||
| #endif | ||
|
|
||
| /*++ | ||
| Function : | ||
| CONTEXTToNativeContext | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.