Skip to content

fix(hip): skip the fused-path BF16-only marks when GGML_CUDA_DISABLE_FUSION is set - #834

Merged
davide221 merged 1 commit into
Luce-Org:mainfrom
Graffioh:fix/hip-disable-fusion-bf16-marks
Oct 10, 2026
Merged

davide221 merged 1 commit into
Luce-Org:mainfrom
Graffioh:fix/hip-disable-fusion-bf16-marks

Conversation

@Graffioh

@Graffioh Graffioh commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

Why

GGML_CUDA_DISABLE_FUSION=1 is the switch we reach for when output looks wrong and we want to rule out a fused kernel. On Strix Halo (gfx1151, RDNA3.5) with a qwen4exp model it currently can't be used: every prefill of 512+ rows aborts in ggml_cuda_compute_forward with

MMB: MUL(...) reads BF16-only tensor ... through the unfused path
MMB: ... writes a BF16/F16-only tensor through the unfused path

Cause

Before each graph runs, the qwen4exp HC marking pass in ggml_backend_cuda_graph_compute flags some hyper-connection intermediates as BF16-only, so that they are stored only as BF16 and never in F32:

  • the HC_COMBINE_NORM xn view. One of the consumers it accepts is the xn*gate MUL, which ggml_cuda_hc_gate_mix fuses and which reads xn through the BF16 cache.
  • the gate-mix dst, which is written as BF16 in place by ggml_cuda_hc_gate_mix.

Both marks assume ggml_cuda_hc_gate_mix runs, but that kernel is only reached inside the !disable_fusion block. With fusion off, the plain MUL and gate-mix ops touch the marked tensors, and the guard in compute_forward aborts, as it should.

The routed-GLU pass in the same block already accounts for this with a fusion_off check. The two passes above were missed.

Change

Hoist the existing fusion_off flag to the top of the block and skip the xn and gate-mix marking passes when it is set. Unmarked tensors take the normal F32 path. The routed down-row F16 mark stays as it is: its only reader is HC_COMBINE_NORM itself, not a fused kernel.

The default path (fusion on) is unchanged. The flags are evaluated once and the marking runs exactly as before.

Testing

  • ggml-cuda.cu compiles for gfx1151 (GGML_USE_HIP)
  • gfx1151 + qwen4exp, GGML_CUDA_DISABLE_FUSION=1, prefill ≥512 rows: aborts before, runs after
  • Default path: output identical before and after

The on-device checks are pending: there's no qwen4exp model on the test box yet. This PR stays in draft until they're done.

🤖 Generated with Claude Code

View guided diff

…FUSION is set

On RDNA3.5 qwen4exp graphs the HC marking pass flags the combine-norm xn view and
the gate-mix dst as BF16-only. Both marks rely on ggml_cuda_hc_gate_mix, which only
runs when fusion is enabled; with GGML_CUDA_DISABLE_FUSION=1 the unfused xn*gate MUL
and gate-mix ops reach compute_forward's guard and every prefill of 512+ rows aborts
("MMB: ... reads/writes a BF16-only tensor ... through the unfused path").

Hoist the existing fusion_off flag (already used by the routed-GLU pass) and apply it
to those two passes too. The default fusion-on path is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Graffioh
Graffioh marked this pull request as ready for review October 10, 2026 06:33

@davide221 davide221 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review

No issues found. Checked for bugs and logic errors in the changed code.

Reviewed 05515fb8 · 0 candidate issues checked, kept at ≥70% confidence · add the code-review label to review again

@davide221
davide221 merged commit ae039d0 into Luce-Org:main Oct 10, 2026
12 checks passed
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