Skip to content

feat(shader): support bindless image table stores - #2500

Closed
Scramblrrrr wants to merge 1 commit into
boykopovar:mainfrom
Scramblrrrr:codex/bindless-image-stores
Closed

Scramblrrrr wants to merge 1 commit into
boykopovar:mainfrom
Scramblrrrr:codex/bindless-image-stores

Conversation

@Scramblrrrr

@Scramblrrrr Scramblrrrr commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What

A shader can load a T# through a descriptor heap and then issue IMAGE_STORE to the selected image. The planner currently rejects all storage-image tables; the write emitter also only accepts a fixed descriptor. This stops 007 First Light registration with bindless storage image tables are unsupported.

Accept non-atomic storage tables in resource planning and resolution. For stores, combine the execution predicate with the table's mapped predicate, load the selected entry's runtime heap index, and decorate the dynamic index, descriptor pointer and image for non-uniform indexing when required. The existing per-image storage bindings and pending-write/readback tracking remain in use. Dynamic mip writes take the selected entry's element count rather than the table root's specialization. No resource capacity is raised, and shader validation remains enabled.

Table atomics, packed accesses and BY2/BY4 remain explicit errors. This changes descriptor selection and write gating; it does not add new arithmetic or IMAGE_STORE instruction semantics.

The synthetic Vulkan regression checks:

  • Two different table keys write distinct values to distinct textures, with explicit pending-write flush/readback and every untouched texel checked.
  • A null entry, the out-of-range key 0xffffffff and a disabled branch preserve both destinations.
  • Changing keys reuses the same compiled artifact.
  • Two wave32 groups in a 64-thread workgroup select different textures and write disjoint texels; a wave64 program checks all 64 texels on a 32-wide host.

Restoring the original storage-table rejection makes the regression fail. With the implementation it passes. The captured title request (104 code words, wave64, 32x32 workgroup) also prepares and validates into 130,490 SPIR-V words. No title shaders, serialized requests, executables or assets are included.

Tested

Rebased onto upstream a29ef592190d0450eacbde15ec8e228e379ed753; branch head 92240a41838e4f84404a8e77a0b90cbd2f28d341. Windows 11, Ryzen 7 7800X3D, RTX 5070 Ti, Release MinGW GCC 15.2, SPIRV-Tools OFF.

The relevant checks (agc_driver_bindless_image_stores) pass in a combined validation checkout containing the independently rebased reciprocal, shared-loop-exit, vertex-fetch-aliasing, table-store, SRT-depth, constant-phi and deferred-specialization topics. All 10 selected CTest cases passed in 2.31 seconds, with Vulkan required for the table-store execution regression. Each public branch contains its own topic only; these are combined-checkout results, not seven separate full builds. Conventions and diff checks pass for each branch.

The rebase also updates the fixture to pass the GPU-accessibility flag required by current main's five-argument guest-allocation API. The implementation retains explicit errors for unsupported atomic, packed and BY2/BY4 table accesses. Wide sampled-table runtime work is a separate branch.

No fresh game run, visible frame or playable gameplay is claimed for these rebased heads. Earlier reduced-regression and local-title observations motivated the original change; title code and captures remain local. Linux and macOS were not rerun locally for this rebase.

Checklist

  • Based on upstream main a29ef592; one topic per branch
  • No new comments in code
  • Unimplemented paths retain explicit errors
  • No investigation files, title data or images in the repository
  • No new third-party code
  • Generic behavior
  • Depends on: none for this correction
  • AI-assisted: yes, prepared with Codex; author is responsible for review

GitHub refuses reopening this existing PR with state cannot be changed. The branch was force-pushed or recreated. The public topic branch above contains the rebased/tested head; this closed PR retains its historical head. No replacement upstream PR was opened because current intake still applies automatically to newly opened PRs.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Thank you for your interest in the project! We are sorry, but we cannot accept more PRs while we work through the existing backlog. This PR has been automatically closed and labeled "intake paused". This is not a judgment of the quality of your changes.

@Scramblrrrr

Copy link
Copy Markdown
Contributor Author

Rebased this branch again onto current main 3cb317f after GitHub reported a new conflict. Head: f80168e. The conflict was only the appended TechnicalDebt entries; both entries are preserved. The earlier Windows/Vulkan results apply to the previous base, not this new head; focused revalidation is pending. This PR remains closed under the current intake policy.

@Scramblrrrr

Copy link
Copy Markdown
Contributor Author

Focused validation of rebased head f80168e on main 3cb317f is complete: agc_driver_bindless_image_stores passed on RTX 5070 Ti with ANYPS5_REQUIRE_VULKAN=1, and agc_spirv_module passed. Windows GCC 15.2 Release, SPIRV-Tools optimization OFF. The branch remains closed; no duplicate upstream PR or intake bypass was created.

@Scramblrrrr

Copy link
Copy Markdown
Contributor Author

Current-main rebase available on the existing fork branch: 9890612, base a11c8f7. Only the appended TechnicalDebt section conflicted; both main entries and the topic entry are retained. Implementation changes merged cleanly.

Fresh Windows Release/GCC 15.2 build with SPIRV-Tools OFF: agc_driver_bindless_image_stores (required Vulkan, RTX 5070 Ti) and agc_spirv_module, 2/2 pass in 1.15 s. Conventions and diff checks pass. This is the standalone topic checkout, not the earlier combined integration build. No new title result or full suite claim. This closed PR retains its historical GitHub head; updated source is codex/bindless-image-stores. No duplicate upstream PR or intake workaround. AI-assisted (Codex).

@Scramblrrrr

Copy link
Copy Markdown
Contributor Author

Overlap review: latest #757 (e904eb1) now includes runtime storage-table emission and explicit store regressions (RunStoreTests); #758 (1ff81c2) adds pointer tables. Both merge cleanly onto current main a11c8f7 as our public cross-test d71402c. I am testing that stack before proposing any replacement for this historical closed PR. Our independent rebased topic remains available at 9890612 with its standalone Vulkan results above; no duplicate table implementation is being opened while this overlap is evaluated. AI-assisted (Codex).

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

Labels

intake paused Closed while PR intake is limited to authors with at least 10 PRs merged into main needs rebase Conflicts with main, rebase needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant