Skip to content

codegen: disable broken PostRADualIssue pass (miscompiles on Maxwell)#4

Open
HueponiK wants to merge 1 commit into
devkitPro:masterfrom
HueponiK:disable-broken-postradualissue
Open

codegen: disable broken PostRADualIssue pass (miscompiles on Maxwell)#4
HueponiK wants to merge 1 commit into
devkitPro:masterfrom
HueponiK:disable-broken-postradualissue

Conversation

@HueponiK
Copy link
Copy Markdown

@HueponiK HueponiK commented Jun 4, 2026

Summary

PostRADualIssue is an experimental post-RA peephole pass that reorders
instructions to pack dual-issue pairs. Its commutation-legality check
(isChainedCommutationLegal()Instruction::isCommutationLegal()) misses a
dependency, so it can move an instruction across a producer of a value it
reads. The dual-issued instruction then consumes a stale value, producing
an intermittent, timing-dependent miscompile.

This PR disables the pass (it only runs at optLevel >= 2).

Symptom / reproduction

Video of the issue:
https://youtu.be/5T3hu8OA_C0

On a real Tegra X1 (Maxwell, sm_53), the miscompile shows up as dancing black
"knife-cut" artifacts in heavy multi-pass slang shaders — reproduced with
the crt-guest-advanced "deconvergence" final pass running on a deko3d backend.

The fault was isolated on-device with a per-pass disable mask over
optimizePostRA/optimizeSSA:

Config Result
All level-2 passes on artifacts
Only PostRADualIssue disabled clean
Only PostRADualIssue enabled (rest off) artifacts

So the artifact tracks PostRADualIssue specifically, not the optimization
level in general.

Why disabling is the right call

PostRADualIssue is not part of mainline mesa/nouveau, which performs no
dual-issue scheduling. It was imported from karolherbst's never-merged
dual_issue_v3 branch plus local tweaks. Disabling it restores the
known-good mainline behaviour; the level-2 SSA passes still provide the
instruction-count reduction.

A complete fix would instead harden isChainedCommutationLegal() to account
for the missing dependency; until someone does that, disabling the pass is the
safe choice.

References

There's quite a chain I got there:
https://codeberg.org/hueponik/goosestation-builder - libretro core. It includes needed fixes as patches. Hopefully we'll be able to upstream them
libretro/RetroArch#19043 - deko3d driver for RA
HueponiK/RetroArch#1 - shaders for deko3d driver

@HueponiK HueponiK marked this pull request as ready for review June 4, 2026 23:56
@fincs
Copy link
Copy Markdown
Member

fincs commented Jun 5, 2026

Can you provide a minimal test case of a shader that fails to compile properly? Dual issue is something I explicitly intended to support in UAM, and I would very much prefer to merge a PR that actually fixes issues instead of disabling the dual issue support. Looking at the code, it seems it may be possible to replace https://github.com/devkitPro/uam/blob/master/mesa-imported/codegen/nv50_ir_target_gm107.cpp#L294 with if (!a->isCommutationLegal(b)), which has an additional check: https://github.com/devkitPro/uam/blob/master/mesa-imported/codegen/nv50_ir.cpp#L901-L906

Additionally, the overall style and formatting of both the text in the PR as well as the code strongly reminds me of LLM output. Did you fully author this yourself by hand, or were LLMs involved at any point?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants