Skip to content

sycl: fix UE4M3 parsing#25608

Open
clee wants to merge 1 commit into
ggml-org:masterfrom
clee:fix-sycl-ue4m3-parsing
Open

sycl: fix UE4M3 parsing#25608
clee wants to merge 1 commit into
ggml-org:masterfrom
clee:fix-sycl-ue4m3-parsing

Conversation

@clee

@clee clee commented Jul 13, 2026

Copy link
Copy Markdown

Overview

The NVFP4 quantization format stores a scaling factor for every group of 16 weights, packed into a single UE4M3 byte.

The SYCL GPU code was converting these scale values using the E4M3 path, but that's signed, and these are unsigned values. (The CPU backend converts these values correctly; this bug is in the SYCL backend alone.)

Additional information

The unsloth/Qwen3.6-27B-NVFP4 quant, after converting to GGUF, generates coherent output on my B70 instead of gibberish once this patch is applied.

Requirements

I used GLM-5.2 to diagnose this issue and to generate the patch, but I tested the output manually before and after applying the patch by hand to confirm that the fix works on my Intel Arc Pro B70 GPU.

@clee clee requested a review from a team as a code owner July 13, 2026 02:17
@github-actions github-actions Bot added ggml changes relating to the ggml tensor library for machine learning SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language labels Jul 13, 2026

@arthw arthw 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.

It's good job!

Thank you!

@mndodd

mndodd commented Jul 13, 2026

Copy link
Copy Markdown

Reproduced here.

Opens up an opportunity for further optimizations.

Before:

$ llama-bench --offline -hf CodeFault/Nvidia-Qwen3.6-27B-NVFP4-GGUF:A -p 512,2048 -n 128 -r 3 -ngl 999 -ub 512

model size params backend ngl test t/s
qwen35 27B NVFP4 16.62 GiB 27.32 B SYCL 999 pp512 664.33 ± 3.81
qwen35 27B NVFP4 16.62 GiB 27.32 B SYCL 999 pp2048 601.61 ± 57.54
qwen35 27B NVFP4 16.62 GiB 27.32 B SYCL 999 tg128 4.73 ± 0.32

After (oneDNN Weight Decompression):

$ lama-bench --offline -hf CodeFault/Nvidia-Qwen3.6-27B-NVFP4-GGUF:A -p 512,2048 -n 128 -r 3 -ngl 999 -ub 512

model size params backend ngl test t/s
qwen35 27B NVFP4 16.62 GiB 27.32 B SYCL 999 pp512 957.21 ± 2.94
qwen35 27B NVFP4 16.62 GiB 27.32 B SYCL 999 pp2048 896.30 ± 4.25
qwen35 27B NVFP4 16.62 GiB 27.32 B SYCL 999 tg128 9.40 ± 0.02

(preliminary, don't hold me to it.)

fnrcum pushed a commit to fnrcum/llama.cpp that referenced this pull request Jul 13, 2026
Backport of upstream PR ggml-org#25608. NVFP4 stores per-16-weight scales as
unsigned E4M3; the SYCL path decoded them via the signed E4M3 helper,
producing gibberish output for NVFP4 models on Intel Arc (B70). Decode
UE4M3 explicitly (exp==0xF is a valid exponent, not NaN).

Assisted-by: Fable 5 (Cursor)
Co-authored-by: Cursor <[email protected]>
fnrcum pushed a commit to fnrcum/llama.cpp that referenced this pull request Jul 13, 2026
…org#25550 (XIELU)

Assisted-by: Fable 5 (Cursor)
Co-authored-by: Cursor <[email protected]>
@arthw arthw added the merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. label Jul 13, 2026

@ggerganov ggerganov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If there is a problem in this function, it should be first fixed in the CPU implementation and then all backends should follow that. cc @ggml-org/ggml-cuda

@ggerganov ggerganov removed the merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. label Jul 13, 2026
@clee

clee commented Jul 13, 2026

Copy link
Copy Markdown
Author

If there is a problem in this function, it should be first fixed in the CPU implementation and then all backends should follow that. cc @ggml-org/ggml-cuda

I could not reproduce the problem on the CPU-specific backend. I only had the problem with SYCL.

However, in comparing the CPU implementation and my proposed fix, I did uncover one thing: the CPU implementation returns 0.0f for both 0x00 and 0x7F, and I was missing the 0x7F path. Fixed.

@clee clee requested a review from ggerganov July 13, 2026 12:21
The NVFP4 quantization format stores a scaling factor for every group of
16 weights, packed into a single UE4M3 byte.

The SYCL GPU code was converting these scale values using the E4M3 path,
but that's *signed*, and these are unsigned values.
@clee clee force-pushed the fix-sycl-ue4m3-parsing branch from 7ec2e03 to 676129a Compare July 13, 2026 12:24
@mndodd

mndodd commented Jul 13, 2026

Copy link
Copy Markdown

If there is a problem in this function, it should be first fixed in the CPU implementation and then all backends should follow that. cc @ggml-org/ggml-cuda

The CPU implementation matches gguf-py/gguf/quants.py:NVFP4.ue4m3_to_fp32.

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 SYCL https://en.wikipedia.org/wiki/SYCL - GPU programming language

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants