Skip to content

fix(relinker): lower ADCX and ADOX for Rosetta - #7173

Open
MarcZX10 wants to merge 4 commits into
boykopovar:mainfrom
MarcZX10:codex/rosetta-adx-flags-7157
Open

MarcZX10 wants to merge 4 commits into
boykopovar:mainfrom
MarcZX10:codex/rosetta-adx-flags-7157

Conversation

@MarcZX10

@MarcZX10 MarcZX10 commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

What

Lower ADCX and ADOX instructions in --to-rosetta output using baseline ADC while restoring unrelated arithmetic flags. ADCX maps ADC's carry-out to CF; ADOX maps that carry-out to OF, as required by the two independent carry chains. The lowering handles 32-bit and 64-bit register or memory sources, REX extensions, stack-relative operands, and RIP-relative relocation; it rejects RSP as the destination because the trampoline cannot safely change its own stack pointer. Adds decoder, converter, and generated-stub regression coverage, and records the unsupported RSP-destination case in relinker debt.

Tested

  • Linux relinker-only Release build completed.
  • Full CTest suite: 188/188 passed, including generated x86-64 stub execution across both instructions, widths, carry inputs, boundary operands, RSP-relative memory, RSP source, and extended registers.
  • The test includes a native ADX differential check gated by CPUID. This host lacks ADX, so that optional path was skipped.
  • check_conventions.py --base upstream/main passed.
  • pr_overlap.py --branch codex/rosetta-adx-flags-7157 reported no overlap with open PRs; unmergeable shared-file candidates were manually reviewed and no duplicate lowering was found.
  • Apple Silicon Rosetta runtime was not available in this Linux environment, so no claim is made for execution under Rosetta itself.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw; silent stubs are listed under technical debt
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: relinker: lower ADCX/ADOX under --to-rosetta? #7157
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 2780 core/relinker/CMakeLists.txt
core/relinker/codegen/src/x86/Amd64OnlyInstructionMatcher.cpp
PR 5021 core/relinker/CMakeLists.txt
docs/dev/debt/relinker.md
PR 5138 core/relinker/CMakeLists.txt
PR 6044 core/relinker/CMakeLists.txt
PR 6067 core/relinker/CMakeLists.txt
PR 6204 core/relinker/CMakeLists.txt
PR 6314 core/relinker/CMakeLists.txt
PR 6327 docs/dev/debt/relinker.md
PR 6328 docs/dev/debt/relinker.md
PR 6354 docs/dev/debt/relinker.md
PR 6410 core/relinker/CMakeLists.txt
PR 6418 core/relinker/CMakeLists.txt
PR 6419 core/relinker/CMakeLists.txt
PR 6448 core/relinker/CMakeLists.txt
PR 6458 core/relinker/CMakeLists.txt
docs/dev/debt/relinker.md
PR 6466 core/relinker/CMakeLists.txt
PR 6607 core/relinker/CMakeLists.txt
PR 6618 core/relinker/CMakeLists.txt
PR 6681 core/relinker/CMakeLists.txt
PR 6684 core/relinker/CMakeLists.txt
PR 6697 core/relinker/CMakeLists.txt
docs/dev/debt/relinker.md
PR 6702 core/relinker/CMakeLists.txt
PR 6713 core/relinker/CMakeLists.txt
PR 6802 core/relinker/CMakeLists.txt
PR 6879 core/relinker/CMakeLists.txt
PR 6903 core/relinker/CMakeLists.txt
PR 6977 core/relinker/CMakeLists.txt
docs/dev/debt/relinker.md
PR 7361 core/relinker/CMakeLists.txt
PR 7417 core/relinker/CMakeLists.txt
docs/dev/debt/relinker.md
PR 7450 core/relinker/CMakeLists.txt
docs/dev/debt/relinker.md
PR 7453 core/relinker/CMakeLists.txt
PR 7532 core/relinker/CMakeLists.txt
PR 7646 core/relinker/CMakeLists.txt
docs/dev/debt/relinker.md
PR 7651 docs/dev/debt/relinker.md

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Thanks @MarcZX100. The Rosetta gating looks right: both hooks return early unless the target is Rosetta, so x86 codegen is unchanged. The decode, RSP handling and flag-preservation checks look good too, and ADCX is correct.

One blocker on ADOX:

ADOX writes the wrong OF. For ADOX, EmitAdx masks ADC's flags with 0x800, so the result OF is ADC's OF, which is signed overflow. ADOX's OF output is the unsigned carry-out (OF:DEST ← DEST + SRC + OF), and ADC leaves that in CF. Checked against native adox on an ADX host (gcc -madx):

input native ADOX lowering
0x7FFFFFFFFFFFFFFF + 0, OF=1 result 0x8000000000000000, OF=0 (CF preserved) OF=1
0xFFFFFFFFFFFFFFFF + 1, OF=0 result 0, OF=1 OF=0

So any ADOX carry chain (multi-precision add/multiply in bignum, crypto or hashing code) gets wrong sums from the second limb on, silently. A fix: for ADOX, take the carry from bit 0 and move it to bit 11, e.g. replace and scratch, 0x800 with and scratch, 1 + shl scratch, 11.

The test currently encodes the same mistake. AdxLoweringTests.cpp computes the expected ADOX flag as signed overflow (~(d^s) & (d^r) & sign). Its only execution input, 0x7FFF…F + 0 with carry 1, is exactly where signed overflow and unsigned carry differ, so it passes because of the bug. Could you:

  • expect the unsigned carry-out for both ADCX and ADOX;
  • add 0xFFFF…F + 1 and a no-carry case for each width;
  • ideally add a differential check against native adcx/adox when CPUID.(EAX=7,ECX=0):EBX[19] (ADX) is set, over a small grid of (dst, src, CF, OF). The Linux CI runners have ADX.

Once that's on a new head we'll re-review and run full CI.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Follow-up manual overlap review after main advanced: the overlap tool reports unmergeable shared files in #5021, #6044, #6111 and #6314. #6111 changes RDTSCP handling in the same matcher but does not implement ADCX/ADOX; #6314 adds game-patch sources/tests to CMake; #6044 reorganizes CTest registration and its stale branch removes older ANDN entries, so the ADCX/ADOX registrations must be retained if it is integrated; #5021 is a broad AOT change with older CMake/debt edits. I found no duplicate ADCX/ADOX lowering. These are manual file reviews because the bot cannot compare those branches cleanly; a rebase/reconciliation may be needed.

@Destroyers212x

Copy link
Copy Markdown
Contributor

Rosetta results for this PR (63c454a), M1 with macOS 26.6.2, relinker-only Release build with -DCMAKE_OSX_ARCHITECTURES=x86_64, so the tests run under Rosetta: 179/179 passed, including adx_lowering (its execution part runs on macOS x86-64) and to_rosetta.

One semantic problem in ADOX's output flag, though. ADOX writes the unsigned carry-out of DEST + SRC + OF to OF: it is ADCX with OF in place of CF, which is what lets the two carry chains interleave. The lowering here writes the signed overflow of the ADC to OF instead, and the test encodes the same rule (AdxLoweringTests.cpp:146, overflow = ~(d ^ s) & (d ^ r) & sign), so both agree and the test passes.

Two 32-bit cases with OF clear on entry that show it:

ADOX carry out (expected OF) signed overflow (OF this PR produces)
0x56789000 + 0x5d4c306a 0 1
0xffffffff + 0xffffffff 1 0

I ran an independent execution test against this PR's EmitAdx under Rosetta. It covers every destination except RSP, sources 6, the destination itself, 11 and RSP, 32/64-bit, all-zero, all-one and mixed inputs, the memory forms ([rsi], [rsi+16], [rsp+0x40], [rdx+rcx*8], [rbp+16], RIP-relative), and entry flags 0x2, 0x8D7, CF only and OF only. The results and all other flags are right, and ADCX passes every case. ADOX fails 708 cases in both widths, all on OF only. The test is on Destroyers212x/AnyPS5 feat/rosetta-adx-lowering (check() computes the expected flags) if you want to reuse it. I won't open a competing PR.

AI-assisted: yes (Claude Code)

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Fixed in f5e857c48. The ADOX path now takes ADC’s unsigned carry-out from CF and maps it into OF; it no longer copies ADC’s signed overflow. The execution test now checks 32- and 64-bit cases across boundary operands and both carry-in values, and includes a native ADX differential check when CPUID reports ADX support. This host has no ADX, so that optional differential path was skipped here. adx_lowering_tests passes, the full Linux relinker-only CTest suite passes 188/188, git diff --check passes, and conventions pass.

@Destroyers212x

Copy link
Copy Markdown
Contributor

Re-ran f5e857c under Rosetta (M1, macOS 26.6.2, x86-64 relinker-only build): adx_lowering and to_rosetta pass, and the independent execution test that failed 708 ADOX cases on 63c454a now passes every case for both instructions, both widths, all register pairs, the memory forms and CF/OF set and clear on entry.

AI-assisted: yes (Claude Code)

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Follow-up: after the broader run, I made the carry-to-OF shift explicitly encode SHL r64, 11 (C1 /4) and reran the suite on the pushed head. The full Linux CTest suite again passes 188/188; the focused ADX test and conventions also pass. The native ADX differential path remains CPUID-gated and skipped on this host.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for rerunning the independent Rosetta matrix. Your results on f5e857c48 confirm the ADOX carry mapping across the reported cases. I then made the one-bit extraction explicitly use SHL r64, 11 (instead of a rotate that produced the same single-bit result) in 8e83a61a2; the focused test and full Linux CTest suite pass again on that pushed head. Hosted cross-platform relinker jobs are still running.

@TheSovietWaffle

Copy link
Copy Markdown

Windows data point: head 8e83a61a2 as-is, Release, Windows 10 Pro 22H2 (19045), WinLibs GCC 15.2.0 r7, x86-64 host (no Rosetta). adx_lowering passes 5 of 5, including its generated-stub execution checks.

Not checked: the rest of the relinker suite, Rosetta.

AI-assisted: yes (Claude Code).

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for checking the final head on Windows. That confirms the generated-stub execution checks pass there as well; the Windows hosted relinker job is still pending, and your result covers the focused test rather than the full suite.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Hosted relinker CI has now completed on head 8e83a61a2: macOS, Linux and Windows jobs all pass, as do conventions and eligibility. The broader Linux/Windows/register-context/write-watch jobs were skipped by path filters; the separate local full Linux relinker-only suite passed 188/188.

@jotitadev jotitadev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Native ADX run of the CPUID-gated differential, which no run here has exercised yet: the earlier hosts had no ADX (Ivy Bridge Xeon, Rosetta on M1).

Setup: head 8e83a61a2 (merges cleanly into main 195d0d08a), Linux x86-64, i7-8700 (adx in /proc/cpuinfo), GCC 15.2, relinker-only Release.

  • adx_lowering_tests: 3/3 pass, with the hasAdx branch taken. The relinker suite passes 188/188.
  • The native branch catches the bug from the first review on its own. I put AdxLowering.cpp from 63c454aa6 (ADOX OF = ADC signed overflow) back and kept the current test:
    • as is, it fails on the reference check, "ADX lowering produced the wrong carry flag or changed an unrelated arithmetic flag";
    • with that reference flag check also disabled, it fails with "ADX lowering differs from native ADX". So the native comparison runs on ADX hardware and covers ADOX's OF independently of the reference model.

One note on the Windows data point above: the execution section of AdxLoweringTests.cpp is under #if (defined(__linux__) || defined(__APPLE__)) && defined(__x86_64__). So on Windows only the decode and conversion checks run, not the generated-stub execution.

Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.

@MarcZX10

Copy link
Copy Markdown
Contributor Author

Thanks for running the current head on native ADX hardware. This confirms both the reference-flag check and the native differential catch the ADOX carry regression; the Windows scope note is useful too. I have no further validation request.

ycarogabriel-lgtm added a commit to ycarogabriel-lgtm/AnyPS5 that referenced this pull request Oct 18, 2026
…tta (@MarcZX100)

Brings the Rosetta ADX lowering from the open upstream pull request into the fork's main ahead of its review. It and the fork's RDTSCP lowering (boykopovar#6111) each add a Rosetta case at the same point of the AMD64-only matcher; both are kept, RDTSCP first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Destroyers212x

Copy link
Copy Markdown
Contributor

Rosetta results for this PR (8e83a61), M1 with macOS 26.6.2, relinker-only Release build:

  • x86-64 under Rosetta (CMAKE_OSX_ARCHITECTURES=x86_64): 179/179 CTest passed. adx_lowering ran its execution checks: the (__linux__ || __APPLE__) && __x86_64__ guard is active, and the binary run directly prints "ADX lowering tests passed". Rosetta does not report ADX (CPUID.(EAX=7,ECX=0):EBX[19] = 0), so the native ADX differential path was skipped here too.
  • Native arm64: 179/179 CTest passed (adx_lowering runs only the decode and convert checks).

I also wrote a separate execution check under Rosetta, linked against build-x64/core/relinker/librelinker_core.a. Its reference model follows the ADCX/ADOX entries in Intel SDM Vol. 2A: dest + src + CF/OF, the carry-out goes only to CF/OF, and the 32-bit form zero-extends. It covered both instructions, both widths, every destination except RSP, every source register including RSP, and [rsp+8] and [rsp-8] memory sources (the second one is inside the red zone). Each case started with random values in all 16 registers and random CF/PF/AF/ZF/SF/OF. 6480 cases, 0 failures. In every case the destination, CF/PF/AF/ZF/SF/DF/OF and all the other GPRs matched, and the 128 bytes below the original RSP were unchanged.

A gap in the PR's test: in the 32-bit cases every destination value is ≤ 0xFFFFFFFF, so the test doesn't check that bits 63:32 of the destination get cleared. The lowering does clear them (my check started with random upper bits), but a destination like 0xDEADBEEF00000001 in values would cover it.

AI-assisted: yes (Claude Code)

ycarogabriel-lgtm added a commit to ycarogabriel-lgtm/AnyPS5 that referenced this pull request Oct 18, 2026
…tta (@MarcZX100)

Includes the open upstream macOS pull request, as the fork's main does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MarcZX10

Copy link
Copy Markdown
Contributor Author

Added 0xDEADBEEF00000001 to both the generated-stub and CPUID-gated native differential operand matrices. In the 32-bit cases it supplies nonzero upper destination/source bits while the expected result remains zero-extended, so those checks now catch a failure to clear bits 63:32. The focused Linux test passes (1/1); conventions and git diff --check pass. Commit: 010ce9b02.

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 18, 2026
ycarogabriel-lgtm added a commit to ycarogabriel-lgtm/AnyPS5 that referenced this pull request Oct 18, 2026
Keeps the macOS port current with 117 upstream commits. Upstream took boykopovar#6111's RDTSCP lowering; the AMD64-only matcher keeps it and the fork's ADCX/ADOX case (boykopovar#7173) after it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ycarogabriel-lgtm added a commit to ycarogabriel-lgtm/AnyPS5 that referenced this pull request Oct 18, 2026
Brings the pull request up to the 117 upstream commits that made it conflict. Upstream took boykopovar#6111's RDTSCP lowering; the AMD64-only matcher keeps it and the ADCX/ADOX case from boykopovar#7173 after it, as on the fork's main, where the suites passed with this merge.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build docs fix error Closes an issue (Fixes #N) needs rebase Conflicts with main, rebase needed relinker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

relinker: lower ADCX/ADOX under --to-rosetta?

5 participants