Repository navigation
Conversation
Preserve the 256-column path and add a native block-32 fallback. Align IQ4_NL index-stride support with the kernel and fix the generic paired-output loop step. Add regression coverage and operator capability records. The original block-32 helper/fallback approach is by Aristo94, from e8e8f9d. Assisted-by: Codex
github-actions
Bot
added
documentation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new shared transposed-index tests trigger SYCL's stride assertion and abort its test run.
1 open finding
What changed in this PR
Extends CUDA IQ4_NL row lookup support while preserving the optimized path for 256-aligned widths.
Changes:
- Adds a 32-value fallback and supports element-aligned index strides.
- Fixes overlapping writes in paired-output iteration.
- Adds regression cases and updates capability documentation.
| File | Description |
|---|---|
| tests/test-backend-ops.cpp | Adds row-width, repeated-index, and transposed-index cases. |
| ggml/src/ggml-cuda/ggml-cuda.cu | Updates IQ4_NL support checks. |
| ggml/src/ggml-cuda/getrows.cu | Adds fallback dispatch and fixes iteration stride. |
| ggml/src/ggml-cuda/dequantize.cuh | Adds block-32 IQ4_NL dequantization. |
| docs/ops/CUDA.csv | Records support for new cases. |
| docs/ops.md | Documents supported widths and index layouts. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Assisted-by: Codex
github-actions
Bot
added
the
SYCL
…-nl-get-rows Assisted-by: Codex # Conflicts: # tests/test-backend-ops.cpp
Assisted-by: Claude Code
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Extends CUDA
GET_ROWSsupport for validIQ4_NLrow widths that are multiples of 32 but not 256. Keeps the existing optimized path for 256-aligned widths.Makes typed I32 index-stride support agree with the kernel requirements. A transposed-index case previously advertised support and then asserted; it passes with this change. Also fixes the generic paired-output loop step, which allowed overlapping writes on longer rows.
Adds regression cases and updates the operator capability records. Credit to Aristo94 for the original block-32 helper/fallback approach in
e8e8f9dcb3f5c0cdd259233503a57a5220e98b7c.Additional information
Validation at
5cdadfc326163ed1fa66cf609fb1761f7928900c:GET_ROWS: 286/286 passed.IQ4_NLcases; 24 unsupported cases skipped.On an RTX 5090, warmed ABBA benchmarks with identical BGE weights and 384-column
IQ4_NLtoken embeddings improved mean throughput 18.9% at pp128 and 4.1% at pp512, with 10 samples per profile/configuration. These results apply to those configurations; they do not establish an AMD speedup. Direct runtime node placement was inferred from the buffer override and backend support conditions, rather than captured in a trace.Related work: #25962 added quantized
GET_ROWSbut retains the 256-columnIQ4_NLrestriction. #29683 fixes short-row dequantization inconvert.cu, a separate path.Requirements