Skip to content

webgpu: add DUP support - #30002

Closed
yomaytk wants to merge 1 commit into
ggml-org:masterfrom
yomaytk:add-dup
Closed

yomaytk wants to merge 1 commit into
ggml-org:masterfrom
yomaytk:add-dup

Conversation

@yomaytk

@yomaytk yomaytk commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Overview

This PR adds dup support to WebGPU, which fixes the following CI error by the new code of #29622 on my M5 (the same test failed with a Bus error on my M5, but it passes with this change).

https://github.com/ggml-org/llama.cpp/actions/runs/37244334061/job/111559110058

In the following llama-graph.cpp code (line 2488), this error seems to be rooted in the fact that the DUP nodes are assigned to the CPU and WebGPU tries to bind the CPU buffer on the GPU side.

inps[2] = ggml_set_rows(ctx0, ggml_dup(ctx0, inp->mixed_embd), build_tok(inp->mixed_tokens), inp->mixed_slots); 

Additional information

Requirements

@yomaytk

yomaytk commented Oct 5, 2026 •

Copy link
Copy Markdown
Member Author

Unfortunately, I temporarily cannot access my local NVIDIA machine, so I'll check it using CI. I'll mark this as ready once CI passes.

Edit: Models Backend Check / webgpu-nvidia passes.

@github-actions github-actions Bot added ggml changes relating to the ggml tensor library for machine learning WebGPU labels Oct 5, 2026
@yomaytk
yomaytk marked this pull request as ready for review October 5, 2026 16:17
@yomaytk
yomaytk requested a review from a team as a code owner October 5, 2026 16:17
@yomaytk

yomaytk commented Oct 5, 2026

Copy link
Copy Markdown
Member Author

@ggerganov I confirmed that #28075 passes test-llama-archs on my M5 without this change, so it could fix similar errors even when the WebGPU backend lacks support for some ops. But, it cannot pass CI for now, and there might be a better approach, so I'll work on it again later.

@yomaytk

yomaytk commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

Closing since this was superseded by #30216.

@yomaytk yomaytk closed this Oct 9, 2026
@yomaytk
yomaytk deleted the add-dup branch October 9, 2026 10:03
@ServeurpersoCom

Copy link
Copy Markdown
Contributor

Closing since this was superseded by #30216.

Sorry @yomaytk, I hadn't seen your PR: it is the same change apart from the order of the cases, so I would have simply approved yours. Thanks for taking it so kindly, I'll look for open PRs first next time.

@ggerganov

Copy link
Copy Markdown
Member

Yes, I also missed this PR.

@ServeurpersoCom

Copy link
Copy Markdown
Contributor

I test #30219 because this is the next "Fix CI" to implement, but this time I checked to make sure a PR didn't already exist 😂

@yomaytk

yomaytk commented Oct 9, 2026

Copy link
Copy Markdown
Member Author

Sorry @yomaytk, I hadn't seen your PR: it is the same change apart from the order of the cases, so I would have simply approved yours. Thanks for taking it so kindly, I'll look for open PRs first next time.

Ah, it’s OK! I should have used a clearer PR title mentioning the CI fix :)

@ServeurpersoCom

Copy link
Copy Markdown
Contributor

No worries at all! And it's even better than a CI fix, since it enables the DUP op on the backend.

For #30219, I went through the dispatch sites: you moved the whole class of row and element wise ops to the 2D grid. Asking an LLM for a CI fix is a double edged sword: by default it patches the failing site and forgets the other entry points, which is exactly what your PR avoids. Once the format line is fixed and it's rebased, we should get a fully green CI!

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

Labels

ggml changes relating to the ggml tensor library for machine learning WebGPU

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants