MagicBeanDev/Hookshot

Archived

Watch

Fork 0

fix(pipelines): Match predicted vertex input numeric types #2248

Closed

klimovich008 wants to merge 4 commits from codex/predict-input-types298 into main

klimovich008 commented

2026-11-11 11:27:46 +00:00

(Migrated from github.com)

What this fixes

Shader preparation can predict an invalid pipeline when vertex shaders use the same attribute locations with different numeric types. Vulkan reports VUID-VkGraphicsPipelineCreateInfo-Input-08733 for these guesses.

Why merge

Match float, signed and unsigned 32-bit input classes as well as locations.
Keep exact-module matching for unfamiliar interfaces; recorded pipelines and draws stay unchanged.
Preserve cache formats, shader instructions and defaults.

Change

gpu/vk/vk_predict.cpp: partition fallback donors by numeric input signature.
tests/test_predict_numeric_inputs.py: compile the real predictor with synthetic interfaces.
docs/tested-configurations.md: record preparation and gameplay checks.

Tested

python -I -B tests/run_all.py --jobs 1 --strict predict_numeric_inputs predict_sees_loaded: numeric classes, canonical IDs, unknown/grouped interfaces and exact matching.
Fails on the old code, passes now. Full suite: 341 passed; four failures reproduced on unchanged main; 30 skipped.
RX 9070 XT: 10 validation reports become zero; 1,407 recorded graphics and 862 compute selections remain. A 95-second Hyrule run shows intact terrain/HUD and no fatal error. No game data is included.

Not covered

Predictions fall from 276 to 4 (208 to 4 program variants); the stutter impact is unmeasured. Fragment-output/attachment numeric matching remains unchanged.
Tested on AMD with matched API14 binaries; API15 gameplay, Linux and NVIDIA untested. Gameplay validation was off.

## What this fixes Shader preparation can predict an invalid pipeline when vertex shaders use the same attribute locations with different numeric types. Vulkan reports `VUID-VkGraphicsPipelineCreateInfo-Input-08733` for these guesses. ## Why merge - Match float, signed and unsigned 32-bit input classes as well as locations. - Keep exact-module matching for unfamiliar interfaces; recorded pipelines and draws stay unchanged. - Preserve cache formats, shader instructions and defaults. ## Change `gpu/vk/vk_predict.cpp`: partition fallback donors by numeric input signature. `tests/test_predict_numeric_inputs.py`: compile the real predictor with synthetic interfaces. `docs/tested-configurations.md`: record preparation and gameplay checks. ## Tested - `python -I -B tests/run_all.py --jobs 1 --strict predict_numeric_inputs predict_sees_loaded`: numeric classes, canonical IDs, unknown/grouped interfaces and exact matching. - Fails on the old code, passes now. Full suite: 341 passed; four failures reproduced on unchanged main; 30 skipped. - RX 9070 XT: 10 validation reports become zero; 1,407 recorded graphics and 862 compute selections remain. A 95-second Hyrule run shows intact terrain/HUD and no fatal error. No game data is included. ## Not covered - Predictions fall from 276 to 4 (208 to 4 program variants); the stutter impact is unmeasured. Fragment-output/attachment numeric matching remains unchanged. - Tested on AMD with matched API14 binaries; API15 gameplay, Linux and NVIDIA untested. Gameplay validation was off.

hckrman101 (Migrated from github.com) reviewed

2026-11-11 12:19:15 +00:00

hckrman101 (Migrated from github.com) left a comment

Thanks, this reads correct and conservative to me, and CI is green. A few things below, one of which I'd like numbers for before merge.

What looks right: anything spirv_vertex_inputs can't classify (16/64-bit, matrices, structs, decoration groups, component-packed or overlapping locations) falls back to exact-module matching instead of being guessed; S32 and U32 are kept apart as Vulkan requires; mesh programs and vertex shaders without generic inputs still share Class{0} as before; recorded pipelines and the draw path are untouched; and the fixture compiles the real predictor.

Minor: tested on AMD with API14 binaries; main is at 15 now (this file isn't affected).

Thanks, this reads correct and conservative to me, and CI is green. A few things below, one of which I'd like numbers for before merge. What looks right: anything `spirv_vertex_inputs` can't classify (16/64-bit, matrices, structs, decoration groups, component-packed or overlapping locations) falls back to exact-module matching instead of being guessed; S32 and U32 are kept apart as Vulkan requires; mesh programs and vertex shaders without generic inputs still share `Class{0}` as before; recorded pipelines and the draw path are untouched; and the fixture compiles the real predictor. Minor: tested on AMD with API14 binaries; main is at 15 now (this file isn't affected).

gpu/vk/vk_predict.cpp

Programs whose vertex inputs come back unclassified (vs_inputs empty) used to get the location-mask fallback and now get no prediction at all unless that exact module was recorded. Each lost prediction can become a compile during play. Could you post the prepare log's predicted N ... made M counts on main vs this branch (same list, same GPU), so we can see how many predictions this drops besides the invalid ones?

Programs whose vertex inputs come back unclassified (`vs_inputs` empty) used to get the location-mask fallback and now get no prediction at all unless that exact module was recorded. Each lost prediction can become a compile during play. Could you post the prepare log's `predicted N ... made M` counts on main vs this branch (same list, same GPU), so we can see how many predictions this drops besides the invalid ones?

gpu/vk/vk_predict.cpp

The fragment side has the same gap: colour outputs are still matched by location only (fsig[f.fs_locs]), so an integer output can still be paired with a float attachment (or the other way round) from a donor. It isn't VUID-08733, but it's the same class of invalid guess. Worth the same numeric-class treatment here, or a note that it's left for later.

The fragment side has the same gap: colour outputs are still matched by location only (`fsig[f.fs_locs]`), so an integer output can still be paired with a float attachment (or the other way round) from a donor. It isn't VUID-08733, but it's the same class of invalid guess. Worth the same numeric-class treatment here, or a note that it's left for later.

tests/test_predict_sees_loaded.py

Nit: this check isn't about the prediction seeing loaded programs, so it might read better as its own tests/test_predict_numeric_inputs.py. The fixture's one-line style (trial(...) and the op(...) chains) is also denser than the rest of the tests.

Nit: this check isn't about the prediction seeing loaded programs, so it might read better as its own `tests/test_predict_numeric_inputs.py`. The fixture's one-line style (`trial(...)` and the `op(...)` chains) is also denser than the rest of the tests.

klimovich008 (Migrated from github.com) reviewed

2026-11-11 13:10:15 +00:00

gpu/vk/vk_predict.cpp

The paired prepare logs on the same RX 9070 XT and recorded scene list show:

	Main control	This change
Predicted pipelines / made	276 / 276	4 / 4
Program variants receiving predictions	208	4
Recorded graphics pipeline selections	1,407	1,407
Compute pipeline selections	862	862
VUID-08733 error reports	10	0

The raw list, notes, compute keys and starting caches were identical. The prediction reduction is substantial; the ten validation reports do not prove that all 272 removed guesses were invalid. I have not measured a stutter benefit and am checking unclassified interfaces versus numeric-class mismatches before treating this as ready to merge. These measurements use the matched API14 binaries stated in the description.

The paired prepare logs on the same RX 9070 XT and recorded scene list show: | | Main control | This change | |---|---:|---:| | Predicted pipelines / made | 276 / 276 | 4 / 4 | | Program variants receiving predictions | 208 | 4 | | Recorded graphics pipeline selections | 1,407 | 1,407 | | Compute pipeline selections | 862 | 862 | | VUID-08733 error reports | 10 | 0 | The raw list, notes, compute keys and starting caches were identical. The prediction reduction is substantial; the ten validation reports do **not** prove that all 272 removed guesses were invalid. I have not measured a stutter benefit and am checking unclassified interfaces versus numeric-class mismatches before treating this as ready to merge. These measurements use the matched API14 binaries stated in the description.

klimovich008 (Migrated from github.com) reviewed

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

gpu/vk/vk_predict.cpp

Agreed. Fragment output numeric types are still matched by location here; this PR does not fix incompatible fragment-output/attachment guesses. I am leaving that as a separate fix and will make the limit explicit in the description. The zero-VUID result applies only to the tested recorded subset.

klimovich008 (Migrated from github.com) reviewed

2026-11-11 13:36:08 +00:00

gpu/vk/vk_predict.cpp

A follow-up check with the actual C++ spirv_vertex_inputs helper classified all 68 sampled real vertex modules; none took the unknown-interface path. An inspected failing donor/target pair is classifiable on both sides but disagrees on float versus unsigned-integer input type at the same location.

That supports numeric partitioning as a reason for the reduction, rather than widespread parser rejection in this sample. It does not attribute every omitted prediction or establish the stutter cost. Restoring location-only guesses would reintroduce the reproduced validation failure.

A follow-up check with the actual C++ `spirv_vertex_inputs` helper classified all 68 sampled real vertex modules; none took the unknown-interface path. An inspected failing donor/target pair is classifiable on both sides but disagrees on float versus unsigned-integer input type at the same location. That supports numeric partitioning as a reason for the reduction, rather than widespread parser rejection in this sample. It does not attribute every omitted prediction or establish the stutter cost. Restoring location-only guesses would reintroduce the reproduced validation failure.

klimovich008 (Migrated from github.com) reviewed

2026-11-11 14:25:39 +00:00

tests/test_predict_sees_loaded.py

Moved the numeric-input fixture into tests/test_predict_numeric_inputs.py and expanded its C++ formatting in 5d34f63e. The loaded-program visibility test is restored to its original purpose. The focused checks pass; the numeric test compiles and fails on unchanged main. Full local suite: 341 passed, the same four failures previously reproduced on the unchanged base, 30 skipped.

Moved the numeric-input fixture into `tests/test_predict_numeric_inputs.py` and expanded its C++ formatting in 5d34f63e. The loaded-program visibility test is restored to its original purpose. The focused checks pass; the numeric test compiles and fails on unchanged main. Full local suite: 341 passed, the same four failures previously reproduced on the unchanged base, 30 skipped.

klimovich008 commented

2026-11-11 15:41:36 +00:00

(Migrated from github.com)

The refreshed numeric-input fixture and existing loaded-shader check both pass in Linux CI. The failing Linux jobs show Ubuntu mirror DNS failures during package installation, then missing spirv_val/glslang/ccache; AppImage fails at the same package-install step. Windows builder passed. I tried rerunning the failed jobs, but GitHub requires repository admin rights; could a maintainer rerun them?

The refreshed numeric-input fixture and existing loaded-shader check both pass in Linux CI. The failing Linux jobs show Ubuntu mirror DNS failures during package installation, then missing `spirv_val`/`glslang`/`ccache`; AppImage fails at the same package-install step. Windows builder passed. I tried rerunning the failed jobs, but GitHub requires repository admin rights; could a maintainer rerun them?

hckrman101 commented

2026-11-11 16:45:26 +00:00

(Migrated from github.com)

Thanks for the numbers and the follow-up check on the parser, that settles where the reduction comes from.

Heads-up on an overlap: #2323 also stops the prediction from making pipelines whose vertex input types don't match the module (the same VUID-08733 class). It reads each module's input types and checks every candidate layout against them; when the most common layout for a signature doesn't fit, it takes the most common one that does, so variants still get a pipeline rather than none. On the shipped list here (NVIDIA) that removes 8,209 wrong-type predictions, and none of the 15,330 recorded draws fail the check. Both PRs change the same part of vk_predict.cpp, so only one of them should land.

What #2323 doesn't have yet is a validation-layer count, which you have set up. Would you be able to run #2323's head on the RX 9070 XT with validation on, on the same scene list, and post the VUID-08733 count and the predicted/made numbers? If it still produces 08733 errors, I'd fold your stricter check into #2323 for the cases it misses. Fragment-output numeric matching stays a gap in both, as you noted.

Thanks for the numbers and the follow-up check on the parser, that settles where the reduction comes from. Heads-up on an overlap: #2323 also stops the prediction from making pipelines whose vertex input types don't match the module (the same VUID-08733 class). It reads each module's input types and checks every candidate layout against them; when the most common layout for a signature doesn't fit, it takes the most common one that does, so variants still get a pipeline rather than none. On the shipped list here (NVIDIA) that removes 8,209 wrong-type predictions, and none of the 15,330 recorded draws fail the check. Both PRs change the same part of `vk_predict.cpp`, so only one of them should land. What #2323 doesn't have yet is a validation-layer count, which you have set up. Would you be able to run #2323's head on the RX 9070 XT with validation on, on the same scene list, and post the VUID-08733 count and the predicted/made numbers? If it still produces 08733 errors, I'd fold your stricter check into #2323 for the cases it misses. Fragment-output numeric matching stays a gap in both, as you noted.

hckrman101 commented

2026-11-11 16:48:28 +00:00

(Migrated from github.com)

Closing in favour of #2323, which covers the same wrong-type predictions while keeping a fitting pipeline for each variant. Thanks for the investigation and the AMD validation numbers; the fragment-output gap you found is worth its own PR, and if a validation run of #2323 still shows 08733 errors, your stricter check is the fallback.

Commenting is not possible because the repository is archived.

No reviewers