fix(shaders): two large compute programs lower (an immediate is not R254) and get their local memory #1800

2026-11-10T17:10:49Z

Joey-Magnus commented

2026-11-10 17:10:49 +00:00

(Migrated from github.com)

Two of OoT's largest compute programs (~32,000 instructions each) are refused as "native program uses lowering scratch registers", although they use no scratch register. The lowering mistakes an immediate operand's low byte for register R254. Once they lower, a second bug shows up: compute programs never get their local memory, so the game aborts at their first local access. This PR fixes both. The second commit must land with the first.

1. An immediate's low byte is not a register (lower_to_maxwell.h)
Both stop at LOP3.LUT R231, R231, 0xfffffffe, RZ, 0xc0 (e7e77812 fffffffe 078ec0ff 000fc600; the other has R236 ... 0xfffffffd). With the uniform bank in use, R231 is a high GPR. The first loop over an instruction's fields decides whether it has high GPRs and counts only fields that hold registers (register_field: bit 32 only in the register form or for listed opcodes). The second loop, which marks which GPRs are high, went over every field, including the immediate at bit 32. So 0xfe became "R254", a scratch register, and the program was refused. The second loop now uses the same register_field test (factored into a lambda).

2. Compute programs get their local memory size (vk_shader.cpp)
The Env set local_memory only for graphics stages, from their header. Compute programs got 0, the SPIR-V back end declared no local array, and a compute program that spills (LDL/STL) aborted the game in Sirit::OpAccessChain from EmitLoadLocal (gdb backtrace: seed_compute → translate_into → EmitSPIRV → EmitLoadLocal). The size is in the NVN2 control section at +0x68, bytes a thread. I found it by dumping 400 compute control sections:

+0x68 is 0xd80 for these two programs and 0 for 393 of the others.
+0x50 is always the same value times 32 (a warp's).
The programs' constant LDL/STL offsets reach 0xd0c.

parse_code now reads it next to the shared memory size (+0x64).

Tests

New tests/test_lower_immediate_not_register.py: a uniform-address LDG (so the uniform bank is in use), then each LOP3 from the log, then EXIT. Refused on main, lowered here. An immediate below the scratch range lowers either way.
New tests/test_compute_local_memory.py (source check): the compute branch of parse_code reads +0x68, and the Env gives it to compute programs.
run_all.py: all pass except nx2/test_nup_stand_in.py, which is red on main (fixed by #1790).
In game (Linux, RTX 4070 Ti, 595.99.02), stacked on #1769/#1778/#1789: programs that do not lower 4 → 2, the startup build's failed programs 5 → 3, no abort, gameplay renders normally (castle grounds screenshots).

🤖 Generated with Claude Code

Two of OoT's largest compute programs (~32,000 instructions each) are refused as "native program uses lowering scratch registers", although they use no scratch register. The lowering mistakes an immediate operand's low byte for register R254. Once they lower, a second bug shows up: compute programs never get their local memory, so the game aborts at their first local access. This PR fixes both. The second commit must land with the first. **1. An immediate's low byte is not a register** (`lower_to_maxwell.h`) Both stop at `LOP3.LUT R231, R231, 0xfffffffe, RZ, 0xc0` (`e7e77812 fffffffe 078ec0ff 000fc600`; the other has `R236 ... 0xfffffffd`). With the uniform bank in use, R231 is a high GPR. The first loop over an instruction's fields decides whether it has high GPRs and counts only fields that hold registers (`register_field`: bit 32 only in the register form or for listed opcodes). The second loop, which marks *which* GPRs are high, went over every field, including the immediate at bit 32. So 0xfe became "R254", a scratch register, and the program was refused. The second loop now uses the same `register_field` test (factored into a lambda). **2. Compute programs get their local memory size** (`vk_shader.cpp`) The `Env` set `local_memory` only for graphics stages, from their header. Compute programs got 0, the SPIR-V back end declared no local array, and a compute program that spills (LDL/STL) aborted the game in `Sirit::OpAccessChain` from `EmitLoadLocal` (gdb backtrace: `seed_compute` → `translate_into` → `EmitSPIRV` → `EmitLoadLocal`). The size is in the NVN2 control section at **+0x68**, bytes a thread. I found it by dumping 400 compute control sections: - +0x68 is 0xd80 for these two programs and 0 for 393 of the others. - +0x50 is always the same value times 32 (a warp's). - The programs' constant LDL/STL offsets reach 0xd0c. `parse_code` now reads it next to the shared memory size (+0x64). **Tests** - New `tests/test_lower_immediate_not_register.py`: a uniform-address LDG (so the uniform bank is in use), then each LOP3 from the log, then EXIT. Refused on `main`, lowered here. An immediate below the scratch range lowers either way. - New `tests/test_compute_local_memory.py` (source check): the compute branch of `parse_code` reads +0x68, and the `Env` gives it to compute programs. - `run_all.py`: all pass except `nx2/test_nup_stand_in.py`, which is red on `main` (fixed by #1790). - In game (Linux, RTX 4070 Ti, 595.99.02), stacked on #1769/#1778/#1789: programs that do not lower 4 → 2, the startup build's failed programs 5 → 3, no abort, gameplay renders normally (castle grounds screenshots). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

hckrman101 (Migrated from github.com) reviewed

2026-11-10 17:14:45 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks, both diagnoses look right to me, and the log encodings make this easy to follow. LOP3.LUT 0x812 is form 4, so bit 32 is the immediate, and the first loop already ignores it. The local-memory half matches what Eden does: DefineLocalMemory returns early at size 0, local_memory stays Id 0, and EmitLoadLocal's access chain aborts. The shader stamp is a hash of eden/ + vk_shader.cpp (nx2/build.py), so nothing stale is reused from the program cache.

One real concern, on the second loop (inline). The register_field list doesn't cover every opcode the lowering reads a register from at bit 32. Until now that gap only mattered when bit 32 was the only high field, because the second loop still marked it whenever another field set any_high. With the new continue that fallback is gone, so a few instructions that lowered correctly before now lose their REGMAP for Rb.

Smaller points:

Nit: +0x68 is taken as-is. 393/395 zeros plus the +0x50 == 32 * +0x68 relation is good evidence, but a cheap sanity check (e.g. require that relation, or cap the size and log) would stop a misread field from turning into a huge Private array and a driver compile stall on some other program. Also worth knowing (pre-existing, not this PR): LoadLocal's bounds check is a Select after the load, and STL with a dynamic offset isn't checked at all, so an undersized value means out-of-bounds Private accesses (UB), not clamping.
Nit: test_lower_immediate_not_register.py only checks that the programs lower. A case like the TLD4 one below, asserting that the high Rb still gets its map prefix, would pin down the behaviour that changes here. test_compute_local_memory.py is a source-text check; that's fine for this.
The branch is 98 commits behind main and tests/run_all.py conflicts with the test_lower_tld_ms.py line from #1769 (adjacent lines; it will conflict the same way with #1778's entry). It's a trivial resolve. The .h and .cpp merge cleanly, and there's no overlap with #1778's LDG/STG hunks.
No CI ran on this branch.

Thanks, both diagnoses look right to me, and the log encodings make this easy to follow. `LOP3.LUT` 0x812 is form 4, so bit 32 is the immediate, and the first loop already ignores it. The local-memory half matches what Eden does: `DefineLocalMemory` returns early at size 0, `local_memory` stays Id 0, and `EmitLoadLocal`'s access chain aborts. The shader stamp is a hash of `eden/` + `vk_shader.cpp` (nx2/build.py), so nothing stale is reused from the program cache. One real concern, on the second loop (inline). The `register_field` list doesn't cover every opcode the lowering reads a register from at bit 32. Until now that gap only mattered when bit 32 was the *only* high field, because the second loop still marked it whenever another field set `any_high`. With the new `continue` that fallback is gone, so a few instructions that lowered correctly before now lose their REGMAP for Rb. Smaller points: - Nit: `+0x68` is taken as-is. 393/395 zeros plus the `+0x50 == 32 * +0x68` relation is good evidence, but a cheap sanity check (e.g. require that relation, or cap the size and log) would stop a misread field from turning into a huge Private array and a driver compile stall on some other program. Also worth knowing (pre-existing, not this PR): `LoadLocal`'s bounds check is a `Select` *after* the load, and STL with a dynamic offset isn't checked at all, so an undersized value means out-of-bounds Private accesses (UB), not clamping. - Nit: `test_lower_immediate_not_register.py` only checks that the programs lower. A case like the TLD4 one below, asserting that the high Rb still gets its map prefix, would pin down the behaviour that changes here. `test_compute_local_memory.py` is a source-text check; that's fine for this. - The branch is 98 commits behind main and `tests/run_all.py` conflicts with the `test_lower_tld_ms.py` line from #1769 (adjacent lines; it will conflict the same way with #1778's entry). It's a trivial resolve. The `.h` and `.cpp` merge cleanly, and there's no overlap with #1778's LDG/STG hunks. - No CI ran on this branch.

gpu/vk/eden/shader_recompiler/frontend/ampere/lower_to_maxwell.h

This continue changes output for instructions that lowered correctly before. register_field is false at bit 32 for several opcodes whose lowering reads Rb from there: TLD4 0xb63/0xf63 (source_b), TMML 0xb69/0xf69, TEX 0xf60, SHFL.IDX 0x589 (form 2, register lane at 32), and STL 0x987 in the first (non-extended) pass. Take TLD4 with uniform bank in use, Ra = R228, Rb = R230. Before, Ra set any_high and this loop also marked R230/R231 native_high, so the MOV that copies Rb into R251 got the map prefix. Now R230 is skipped, and that MOV reads the plain register while the producer wrote the extended one. That's the same class of bug the ISBEWR/SUST/RED/TXD additions above fixed. It also breaks the "every program that lowered before lowers to the same words" invariant from the extended_renaming comment.

Two options: (a) add those opcodes to register_field (that also closes the old gap where Rb is the only high field), or (b) skip here only when bit 32 is positively an immediate/cbuf (the ALU families operand() reads, forms 2-5), so every other opcode keeps the conservative marking.

This `continue` changes output for instructions that lowered correctly before. `register_field` is false at bit 32 for several opcodes whose lowering reads Rb from there: TLD4 `0xb63`/`0xf63` (`source_b`), TMML `0xb69`/`0xf69`, TEX `0xf60`, SHFL.IDX `0x589` (form 2, register lane at 32), and STL `0x987` in the first (non-extended) pass. Take TLD4 with uniform bank in use, `Ra = R228`, `Rb = R230`. Before, Ra set `any_high` and this loop also marked R230/R231 `native_high`, so the MOV that copies Rb into R251 got the map prefix. Now R230 is skipped, and that MOV reads the plain register while the producer wrote the extended one. That's the same class of bug the ISBEWR/SUST/RED/TXD additions above fixed. It also breaks the "every program that lowered before lowers to the same words" invariant from the `extended_renaming` comment. Two options: (a) add those opcodes to `register_field` (that also closes the old gap where Rb is the only high field), or (b) skip here only when bit 32 is positively an immediate/cbuf (the ALU families `operand()` reads, forms 2-5), so every other opcode keeps the conservative marking.

gpu/vk/vk_shader.cpp

					
				@ -297,2 +309,4 @@

				            c.local_memory = 0;

				        }

				    }

				    c.ok = true;

Nit: maybe sanity-check this before trusting it, e.g. that +0x50 == 32 * +0x68, or cap it and log, so a field misread on some other control section can't produce a multi-MB Private array.

Nit: maybe sanity-check this before trusting it, e.g. that `+0x50 == 32 * +0x68`, or cap it and log, so a field misread on some other control section can't produce a multi-MB Private array.

hckrman101 commented

2026-11-10 17:50:50 +00:00

(Migrated from github.com)

Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!

Joey-Magnus commented

2026-11-10 17:57:18 +00:00

(Migrated from github.com)

Thanks, good catch on TLD4/TMML/TEX/SHFL.IDX/STL. Changed in f58c6c0:

The skip is narrow now. The marking loop skips bit 32 only for an ALU instruction's immediate form (form 4: LOP3/FMUL/IADD3/... with a 32-bit immediate there). Every other field and opcode marks as on main, including your TLD4 Ra = R228, Rb = R230 case. The register_field refactor is gone.
Checked against main on real programs. I dumped 10,730 of OoT's native programs (keys starting 0-3) with RX_VK_DUMP_NATIVE, lowered each with main's lower_to_maxwell.h and with this branch's, and compared the full outputs (instructions + source offsets):
- 10,722 are identical.
- The 2 target programs now lower.
- 6 mesh programs differ only in REGMAP prefix words (0 non-prefix words differ). Example: FMUL R242, R242, 0x3e26c6de (form 4). Main listed R222 (the immediate's low byte 0xde) next to R242 in that instruction's prefix; now only R242. The instruction never reads R222, so the prefix just drops a bogus entry.
Local memory size check (your nit): it's used only if +0x50 holds it times 32 and it's at most 64 KB. Otherwise it's logged and 0. The source test checks that.
Rebased onto main's new run_all.py (the test declares RUN_ALL_ARGS). run_all.py: 263 passed, 0 failed.

Thanks, good catch on TLD4/TMML/TEX/SHFL.IDX/STL. Changed in f58c6c0: - **The skip is narrow now.** The marking loop skips bit 32 only for an ALU instruction's immediate form (form 4: LOP3/FMUL/IADD3/... with a 32-bit immediate there). Every other field and opcode marks as on main, including your TLD4 `Ra = R228, Rb = R230` case. The `register_field` refactor is gone. - **Checked against main on real programs.** I dumped 10,730 of OoT's native programs (keys starting 0-3) with `RX_VK_DUMP_NATIVE`, lowered each with main's `lower_to_maxwell.h` and with this branch's, and compared the full outputs (instructions + source offsets): - 10,722 are identical. - The 2 target programs now lower. - 6 mesh programs differ **only in REGMAP prefix words** (0 non-prefix words differ). Example: `FMUL R242, R242, 0x3e26c6de` (form 4). Main listed R222 (the immediate's low byte 0xde) next to R242 in that instruction's prefix; now only R242. The instruction never reads R222, so the prefix just drops a bogus entry. - **Local memory size check (your nit):** it's used only if +0x50 holds it times 32 and it's at most 64 KB. Otherwise it's logged and 0. The source test checks that. - Rebased onto main's new `run_all.py` (the test declares `RUN_ALL_ARGS`). `run_all.py`: 263 passed, 0 failed.

hckrman101 (Migrated from github.com) approved these changes

2026-11-10 18:05:08 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks for the quick turnaround. This looks good now.

Fixed:

Bit-32 marking. The skip now applies only to form 4 of the ALU families (the three/two lists from RenameableFields, plus ISETP/FSETP). In all of those, operand() reads bit 32 as a 32-bit immediate, so dropping it is correct. Every other opcode is marked the same way as on main, so the TLD4/TMML/TEX/SHFL.IDX/STL Rb case is back to before. ALU opcodes outside the list just keep the old conservative marking, so nothing regresses. Your 10,730-program comparison (identical except 6 REGMAP prefixes that drop a bogus R222) is convincing.
Local memory size. It's only used now when +0x50 == 32 × +0x68 and it's ≤ 64 KB. Otherwise it's logged and set to 0. That's what I was after.
Rebase. It no longer conflicts. #1806 (R2UR) landed after your rebase, but it touches FieldSpan, not this loop, and GitHub reports the PR as mergeable.

Heads-up: #1830 (FIocker's super build, still open) carries both fixes as well. Its version of the first one skips on !register_field, which is the broad form from your first revision and has the TLD4/TMML/... regression. Whichever PR lands second will conflict in lower_to_maxwell.h and vk_shader.cpp. The narrow skip here is the one to keep.

Remaining nits (not blocking):

RUN_ALL_ARGS hardcodes g++ (inline).
Neither test checks that a high Rb at bit 32 in a non-ALU opcode (e.g. TLD4 Ra=R228, Rb=R230) still gets the map prefix. A one-line CHECK on register_map_words or the MOV would lock in the narrowing.
The family list repeats RenameableFields' three/two sets. A small shared helper would keep them from drifting apart.

CI: two checks have passed (F2I immediate, SPIR-V narrow load/store). The rest haven't started yet.

Thanks for the quick turnaround. This looks good now. Fixed: - **Bit-32 marking.** The skip now applies only to form 4 of the ALU families (the `three`/`two` lists from `RenameableFields`, plus ISETP/FSETP). In all of those, `operand()` reads bit 32 as a 32-bit immediate, so dropping it is correct. Every other opcode is marked the same way as on main, so the TLD4/TMML/TEX/SHFL.IDX/STL Rb case is back to before. ALU opcodes outside the list just keep the old conservative marking, so nothing regresses. Your 10,730-program comparison (identical except 6 REGMAP prefixes that drop a bogus R222) is convincing. - **Local memory size.** It's only used now when +0x50 == 32 × +0x68 and it's ≤ 64 KB. Otherwise it's logged and set to 0. That's what I was after. - **Rebase.** It no longer conflicts. #1806 (R2UR) landed after your rebase, but it touches `FieldSpan`, not this loop, and GitHub reports the PR as mergeable. Heads-up: #1830 (FIocker's super build, still open) carries both fixes as well. Its version of the first one skips on `!register_field`, which is the broad form from your first revision and has the TLD4/TMML/... regression. Whichever PR lands second will conflict in `lower_to_maxwell.h` and `vk_shader.cpp`. The narrow skip here is the one to keep. Remaining nits (not blocking): - `RUN_ALL_ARGS` hardcodes `g++` (inline). - Neither test checks that a high Rb at bit 32 in a non-ALU opcode (e.g. TLD4 `Ra=R228, Rb=R230`) still gets the map prefix. A one-line CHECK on `register_map_words` or the MOV would lock in the narrowing. - The family list repeats `RenameableFields`' `three`/`two` sets. A small shared helper would keep them from drifting apart. CI: two checks have passed (F2I immediate, SPIR-V narrow load/store). The rest haven't started yet.

tests/test_lower_immediate_not_register.py

					
				@ -0,0 +13,4 @@

				import tempfile

				from pathlib import Path

				RUN_ALL_ARGS = ["--cxx", "g++"]

Nit: run_all's placeholder would be better here: RUN_ALL_ARGS = ["--cxx", "{cxx}"]. With a literal g++, a machine that only has clang++ (e.g. a Windows setup) skips the test instead of compiling it with the compiler find_tools found.

Nit: run_all's placeholder would be better here: `RUN_ALL_ARGS = ["--cxx", "{cxx}"]`. With a literal `g++`, a machine that only has clang++ (e.g. a Windows setup) skips the test instead of compiling it with the compiler `find_tools` found.

Commenting is not possible because the repository is archived.

No reviewers

hckrman101