closure: add ppc64 ELFv1 support#26749
Conversation
|
@JalonSolov FYI, CI must be green. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a35793e6ce
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } $else $if ppc64 { | ||
| [ | ||
| u8(0x7C), 0x08, 0x02, 0xA6, // mflr %r0 | ||
| 0x48, 0x00, 0x00, 0x05, // bl here | ||
| 0x7D, 0xC8, 0x02, 0xA6, // here: mflr %r14 |
There was a problem hiding this comment.
Keep first closure thunk variant longest
The file-level note says translated/bootstrap C mode (vc/v.c) infers a fixed C array size from the first $if branch, but this new ppc64 thunk is longer than the first ppc64le thunk. On ELFv1 ppc64 bootstrap builds, that mismatch can produce an undersized initializer (or truncated thunk bytes), which breaks closure trampolines at runtime. Please make the first branch the longest one again (or pad it) so bootstrap sizing remains safe.
Useful? React with 👍 / 👎.
|
Some issues with native backend. |
|
Bounce for final check. |
Second step to add
ppc64support for closure.Patch manually tested on x64 and ppc64.
Only ELFv2 has been supported all this time — now and always — and the assembly code for each architecture operated under this memory model. Because of ppc64, additional code had to be written for the closure itself to make it work with ELFv1 as well. All closure tests pass, which is a very good sign to start with.
Note Nr1337 for future me — ppc64 has this peculiarity that in our galaxy there exist versions for two ELF versions. If someone ever requests ppc64 support in ELFv2, add
comptimesupport for elfv1 ($if elfv1 ...) and just reverse the stub fromppc64le. Get access to a system for testing to be sure idea is ok.