Skip to content

vulkan: fix signed FWHT fusion fallback from #349 - #366

Draft
bri-prism wants to merge 2 commits into
prismfrom
fix/pr349-signed-fwht-fallback
Draft

bri-prism wants to merge 2 commits into
prismfrom
fix/pr349-signed-fwht-fallback

Conversation

@bri-prism

Copy link
Copy Markdown
Collaborator

Signed Hadamard fusion can produce incorrect output when the graph allocator overlaps an activation buffer with the final output. The overlap guard cancels fusion by resetting the fused-node count, but the signed-FWHT flag remains set: MUL still runs the fused transform, followed by the original MUL_MAT.

This carries the original optimization from #349, with its author and commit preserved, plus a one-line correction that clears the signed-FWHT flag whenever overlap disables fusion. It is an alternative integration branch for #349, not an additional optimization to merge twice.

Validation:

  • Built the complete Vulkan backend and generated shaders with Apple Clang 21 and shaderc 2026.2.
  • Executed the actual GPU graph on Apple M5 Pro through MoltenVK, using signed FWHT block width 64, activation width 128, and 1/3/5 tokens. Tested disjoint buffers and buffers overlapping by an offset of 256 bytes.
  • Without the correction, all three overlap cases fail against the scalar reference (maximum absolute errors 2.23–2.62), while all disjoint controls pass. With the correction, all six cases pass (maximum absolute error below 2.4e-7).
  • A separate host probe of the production fusion predicate and overlap guard confirms that rejected fusion clears both the fused-node count and dispatch flag.

The original PR's AMD throughput measurements were not repeated. This validation establishes the fallback correction on MoltenVK; it does not establish an end-to-end speedup or coverage of every Vulkan device.

elliotmasheal and others added 2 commits October 11, 2026 11:42
MUL(signs) -> RESHAPE -> MUL_MAT(hadamard) now runs as one dispatch: the
signed FWHT variant applies the sign vector while it loads the row, so the
separate elementwise pass and the barrier that followed it are gone. Mirrors
the Metal signed FWHT fusion.

Ternary-Bonsai-2-27B on RX 6700S (RDNA2), -dev Vulkan1 -ngl 999 -p 512 -n 128 -r 3 -fa 1:
  PTQ1_0  tg128 18.16 -> 24.13
  PQ2_0   tg128 16.88 -> 22.15
PPL 4.2521 +/- 0.29627 and the full test-backend-ops run are unchanged.
Reset the signed-FWHT dispatch flag along with the fused-node count when an overlapping buffer requires the unfused fallback. Otherwise MUL still dispatches the fused transform and the graph also executes the original MUL_MAT.

Validated on Apple M5 Pro through MoltenVK with disjoint and partially overlapping buffers, block width 64, activation width 128, and 1/3/5 tokens. All three overlapping cases fail before this change; all six cases pass after it with maximum absolute error below 2.4e-7 against a scalar reference.
@bri-prism
bri-prism marked this pull request as ready for review October 15, 2026 21:31
@bri-prism
bri-prism marked this pull request as draft October 15, 2026 21:32
@elliotmasheal

Copy link
Copy Markdown

Thanks for integrating this and for keeping the original commit and authorship.

The fix matches what I see: the overlap guard clears the fused-node count and the other flags, but not fused_fwht_signed, so GGML_OP_MUL still runs the fused transform and the original MUL_MAT runs after it.

Please go ahead with #366. I will close #349. Happy to re-run anything on the RDNA2 box if useful.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants