ggml : implement set_rows with i32 index by CISC · Pull Request #16159 · ggml-org/llama.cpp

CISC · 2025-09-21T21:25:26Z

Implements support for I32 index in set_rows, added as many backends as I could.

jeffbolznv · 2025-09-21T21:36:52Z

Vulkan does not currently support this. See this code:

#if defined(SET_ROWS)
#include "generic_binary_head.comp"
layout (binding = 1) readonly buffer C {uvec2 data_i[];};

It's extracting the LSBs of the 64b index using a uvec2. We'd need to add a variant using uint here.

CISC · 2025-09-21T21:37:36Z

Vulkan does not currently support this. See this code:

Yep, just noticed, can you help me implement it?

jeffbolznv · 2025-09-21T21:55:14Z

Sure. Do you want pointers or do you want me to make a PR?

CISC · 2025-09-21T21:56:59Z

Sure. Do you want pointers or do you want me to make a PR?

If you want to make a PR that would be great.

jeffbolznv · 2025-09-21T22:34:45Z

Sure, will do soon.

noemotiovon · 2025-09-22T01:58:13Z

CANN doesn’t support this yet, but adding support shouldn’t be too difficult. I’d be happy to work on it.
Thanks a lot for your contribution and for pointing this out regarding the CANN backend!

noemotiovon · 2025-09-22T02:25:47Z

I found that our kernel already supports index with I32, so there’s no additional work needed.

qnixsynapse

LGTM for SYCL.

ggml/src/ggml-opencl/ggml-opencl.cpp

lhez · 2025-09-22T05:09:05Z

Other than the compiler warnings (see comments), everything looks good for OpenCL.

warnings--

ggml/src/ggml-metal/ggml-metal.metal

ggml/src/ggml-metal/ggml-metal-ops.cpp

ggml/src/ggml-metal/ggml-metal-device.h

ggml/src/ggml-metal/ggml-metal-device.cpp

ggml/src/ggml-metal/ggml-metal.metal

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>

foldl · 2025-09-22T06:52:00Z

Don't let Vulkan down.

theo77186 · 2025-09-22T07:10:04Z

Don't let Vulkan down.

Vulkan is covered by a separate PR: #16162

tests/test-backend-ops.cpp

ggerganov

The Metal changes are good.

tests/test-backend-ops.cpp

JohannesGaessler

The CUDA changes look correct to me but please add a template function to switch either one of the types instead of 2-layered if else statements.

CISC · 2025-09-22T11:28:24Z

The CUDA changes look correct to me but please add a template function to switch either one of the types instead of 2-layered if else statements.

Great suggestion, I'll do that for SYCL too, thanks!

CISC · 2025-09-22T14:05:08Z

Hmmm, why doesn't [no-ci] work anymore?

ggerganov · 2025-09-22T14:21:50Z

Hmmm, why doesn't [no-ci] work anymore?

I think it is [no ci]

reeselevine · 2025-09-22T15:34:20Z

set_rows isn't fully implemented in the WebGPU backend, but I will make sure i32 indexes are supported when it is fully implemented

@danbev

* origin/master: (39 commits) ci : disable AMD workflows + update NVIDIA workflows (ggml-org#16200) ci : enable Vulkan workflow on Mac (ggml-org#16194) ggml-cpu: Respect cpumask settings (ggml-org#16164) ggml : fix uninitialized is_on_grid in quantize_row_iq3_xxs_impl (ggml-org#15928) zdnn: refactor codebase + add docs (ggml-org#16178) codeowners : add @danbev to model-conversion example [no ci] (ggml-org#16190) devops: add s390x containers (ggml-org#15915) ggml-cpu : fix typo in gemm comments [no ci] (ggml-org#16189) feat: Add conversion support in GraniteHybrid for non-hybrid (all attn) (ggml-org#16177) clang-tidy : disable warning about performance enum size (ggml-org#16127) ggml : implement set_rows with i32 index (ggml-org#16159) codeowners : update + cleanup (ggml-org#16174) common : enable `--offline` mode without curl support (ggml-org#16137) webui : fix handling incomplete chunks (ggml-org#16107) embedding : fix typos in README (ggml-org#16171) common : remove unused local variables (ggml-org#16140) ggml : extend ggml_can_fuse to work with non-sequential nodes (ggml-org#16123) ggml : add ggml_op_is_empty (ggml-org#16122) codeowners : update ownership for @ngxson and @allozuar (ggml-org#16128) Vulkan: add conv_transpose_2d operation (ggml-org#16022) ...

* implement set_rows with i32 index * template fix * test quantized path warnings-- * Apply suggestions from code review Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> * forgotten name change * deduplicate cuda/sycl and test-fix * indent++ * vulkan: support set_rows with i32 index type (ggml-org#16162) * disable i32 index for webgpu for now --------- Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> Co-authored-by: Jeff Bolz <jbolz@nvidia.com>

* implement set_rows with i32 index * template fix * test quantized path warnings-- * Apply suggestions from code review Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> * forgotten name change * deduplicate cuda/sycl and test-fix * indent++ * vulkan: support set_rows with i32 index type (#16162) * disable i32 index for webgpu for now --------- Co-authored-by: Georgi Gerganov <ggerganov@gmail.com> Co-authored-by: Jeff Bolz <jbolz@nvidia.com>

implement set_rows with i32 index

a7403b1

CISC requested review from JohannesGaessler, ggerganov, lhez and qnixsynapse September 21, 2025 21:28

template fix

9a7bff3

qnixsynapse approved these changes Sep 22, 2025

View reviewed changes

lhez reviewed Sep 22, 2025

View reviewed changes

ggml/src/ggml-opencl/ggml-opencl.cpp Outdated Show resolved Hide resolved

ggml/src/ggml-opencl/ggml-opencl.cpp Outdated Show resolved Hide resolved

test quantized path

7657ec3

warnings--

ggerganov reviewed Sep 22, 2025

View reviewed changes

CISC commented Sep 22, 2025

View reviewed changes

ggml/src/ggml-metal/ggml-metal.metal Outdated Show resolved Hide resolved

Apply suggestions from code review

480b404

Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>

CISC mentioned this pull request Sep 22, 2025

model : add BailingMoeV2 support #16063

Merged

forgotten name change

9899182

ggerganov reviewed Sep 22, 2025

View reviewed changes

tests/test-backend-ops.cpp Outdated Show resolved Hide resolved

ggerganov approved these changes Sep 22, 2025

View reviewed changes

tests/test-backend-ops.cpp Outdated Show resolved Hide resolved

JohannesGaessler approved these changes Sep 22, 2025

View reviewed changes

deduplicate cuda/sycl and test-fix [no-ci]

aa76826

CISC and others added 2 commits September 22, 2025 16:06

indent++ [no-ci]

bd47cba

vulkan: support set_rows with i32 index type (#16162)

d8abd2d

CISC requested a review from 0cc4m as a code owner September 22, 2025 14:08

github-actions bot added the Vulkan Issues specific to the Vulkan backend label Sep 22, 2025

disable i32 index for webgpu for now [no ci]

2510204

CISC requested a review from slaren as a code owner September 22, 2025 17:06

CISC removed request for 0cc4m and slaren September 22, 2025 17:08

CISC merged commit 3ecb2f6 into master Sep 22, 2025
1 check passed

CISC deleted the cisc/set-rows-i32-idx branch September 22, 2025 17:13

Conversation

CISC commented Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jeffbolznv commented Sep 21, 2025

Uh oh!

CISC commented Sep 21, 2025

Uh oh!

jeffbolznv commented Sep 21, 2025

Uh oh!

CISC commented Sep 21, 2025

Uh oh!

jeffbolznv commented Sep 21, 2025

Uh oh!

noemotiovon commented Sep 22, 2025

Uh oh!

noemotiovon commented Sep 22, 2025

Uh oh!

qnixsynapse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lhez commented Sep 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

foldl commented Sep 22, 2025

Uh oh!

theo77186 commented Sep 22, 2025

Uh oh!

Uh oh!

ggerganov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JohannesGaessler left a comment

Choose a reason for hiding this comment

Uh oh!

CISC commented Sep 22, 2025

Uh oh!

CISC commented Sep 22, 2025

Uh oh!

ggerganov commented Sep 22, 2025

Uh oh!

reeselevine commented Sep 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

CISC commented Sep 21, 2025 •

edited

Loading