Skip to content

metal: fix metal tensor kernel_mul_mm error for large tensor - #28748

Merged
ggerganov merged 2 commits into
ggml-org:masterfrom
SecondSeed:fix/metal-tensor-slice
Oct 12, 2026
Merged

ggerganov merged 2 commits into
ggml-org:masterfrom
SecondSeed:fix/metal-tensor-slice

Conversation

@SecondSeed

Copy link
Copy Markdown
Contributor

Assisted-by: Codex

Overview

In an experiment, I consistently observed that when the offset between the slice address and the tensor base address is greater than or equal to 2 GB, the actual address produced by the Metal Tensor API's slice method differs from the expected address by -4 GB.

This change modifies the kernel_mul_mm implementation in ggml/src/ggml-metal/kernels/mul_mm.metal for the GGML_METAL_HAS_TENSOR == true path. Instead of using the slice method, it explicitly computes the address of each slice and constructs the tensor directly from the computed address.

Related issue: #27784

After this change, the issue is no longer reproducible.

sending long embedding request...
long: output_size=1024 invalid_values=0
 *  Terminal will be reused by tasks, press any key to close it. 

 *  Executing task: python3 scripts/issue27784_probe.py 

sending short embedding request...
short: output_size=1024 invalid_values=0
 *  Terminal will be reused by tasks, press any key to close it.

Additional information

Environment

Hardware: Apple MacBook Air 13 inch M5 16G
macOS: 26.6.2 (25G83)
Metal compiler: Apple metal version 32023.883 (metalfe-32023.883), Target: air64-apple-darwin25.6.0, Thread model: posix

Experiment Design

Each case runs the same matrix multiplication with a K dimension of 16, producing a cooperative tensor cT. The only difference between the two dispatches is the storage destination:

flowchart TB
    A["A: K x tile_rows"] --> MM["Tensor API matmul"]
    B["B: K x tile_columns"] --> MM
    MM --> CT["cT: tile_rows x tile_columns"]
    CT --> P1["Path 1: calculate the dstTile pointer first<br/>then tensor(dstTile, tile shape)"]
    CT --> P2["Path 2: tensor(whole tensor)<br/>then whole.slice(row, column)"]
    P1 --> R1["direct_values"]
    P2 --> R2["slice_expected_values<br/>slice_wrapped_values"]
Loading

Path 1 uses explicit 64-bit address calculation in the host language:

device float * tile = c + (uint64_t) column * m + row;

Path 2 wraps the complete output as a two-dimensional tensor:

auto whole = tensor(c, dextents<int32_t, 2>(m, n), array<int, 2>({1, m}));
cT.store(whole.slice(row, column));

Results

Case Slice start Does the tile cross 2 GiB? Direct path Correct slice address Slice -4 GiB address Conclusion
Start 1024 B below 0x7ffffc00 No 256/256 256/256 0/256 Correct
Start 64 B below 0x7fffffc0 Yes 256/256 256/256 0/256 Correct
Half of tile crosses 0x7ffffe00 Yes 256/256 256/256 0/256 Correct
Exactly at boundary 0x80000000 No 256/256 0/256 256/256 4 GiB early
64 B above 0x80000040 No 256/256 0/256 256/256 4 GiB early
1024 B above 0x80000400 No 256/256 0/256 256/256 4 GiB early
xychart-beta
    title "16 x 16 tile: slice start versus actual write displacement"
    x-axis ["-1024 B", "-64 B", "crossing tile", "0 B", "+64 B", "+1024 B"]
    y-axis "actual address - correct address (GiB)" -4 --> 0
    line [0, 0, 0, -4, -4, -4]
Loading

These results show a step with no transition interval. From 0x7fffffff to 0x80000000, the displacement changes directly from 0 to -4 GiB.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES, I use codex to help me write experiment script.

@SecondSeed
SecondSeed requested a review from a team as a code owner September 11, 2026 10:00
@github-actions github-actions Bot added ggml changes relating to the ggml tensor library for machine learning Apple Metal https://en.wikipedia.org/wiki/Metal_(API) labels Sep 11, 2026
Comment thread ggml/src/ggml-metal/kernels/mul_mm.metal
@ggerganov

Copy link
Copy Markdown
Member

Do you have a test_mul_mat parameters that reproduce the issue with test-backend-ops?

Co-authored-by: YiChen Lv <63285796+forforever73@users.noreply.github.com>
@SecondSeed

SecondSeed commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Do you have a test_mul_mat parameters that reproduce the issue with test-backend-ops?

Yes.

Test cases:

test_cases.emplace_back(new test_mul_mat(
    GGML_TYPE_F32, GGML_TYPE_F32, 16384, 32700, 64, {1, 1}, {1, 1}));
test_cases.emplace_back(new test_mul_mat(
    GGML_TYPE_F32, GGML_TYPE_F32, 16384, 32768, 64, {1, 1}, {1, 1}));
test_cases.emplace_back(new test_mul_mat(
    GGML_TYPE_F32, GGML_TYPE_F32, 16384, 32800, 64, {1, 1}, {1, 1}));
test_cases.emplace_back(new test_mul_mat(
    GGML_TYPE_F32, GGML_TYPE_F32, 16384, 32900, 64, {1, 1}, {1, 1}));

16,384 × 32,768 × 4 = 0x80000000 (2 GiB). The first two cases succeed because the tensor sizes do not exceed 2 GiB, while the last two fail because the tensor sizes exceed 2 GiB.

Test result before the change

ggml_metal_device_init: GPU name:   MTL0 (Apple M5)
ggml_metal_device_init: GPU family: MTLGPUFamilyApple10  (1010)
ggml_metal_device_init: GPU family: MTLGPUFamilyCommon3 (3003)
ggml_metal_device_init: GPU family: MTLGPUFamilyMetal4  (5002)
ggml_metal_device_init: simdgroup reduction   = true
ggml_metal_device_init: simdgroup matrix mul. = true
ggml_metal_device_init: has unified memory    = true
ggml_metal_device_init: has bfloat            = true
ggml_metal_device_init: has tensor            = true
ggml_metal_device_init: use residency sets    = true
ggml_metal_device_init: use shared buffers    = true
ggml_metal_device_init: recommendedMaxWorkingSetSize  = 12713.12 MB
Testing 3 devices

ggml_metal_init: allocating
ggml_metal_init: found device: Apple M5
ggml_metal_init: picking default device: Apple M5
ggml_metal_init: use fusion         = true
ggml_metal_init: use concurrency    = true
ggml_metal_init: use graph optimize = true
Backend 1/3: MTL0
  Device description: Apple M5
  Device memory: 12124 MB (12123 MB free)

ggml_metal_library_compile_pipeline: compiling pipeline: base = 'kernel_mul_mm_f32_f32', name = 'kernel_mul_mm_f32_f32_bci=0_bco=1_ne12=1_ne13=1_r2=1_r3=1'
ggml_metal_library_compile_pipeline: loaded kernel_mul_mm_f32_f32_bci=0_bco=1_ne12=1_ne13=1_r2=1_r3=1      0x74d7e4000 | th_max = 1024 | th_width =   32
  MUL_MAT(type_a=f32,type_b=f32,m=16384,n=32700,k=64,bs=[1,1],nr=[1,1],per=[0,1,2,3],k_v=0,o=1,src_overlap=0): OK
ggml_metal_library_compile_pipeline: compiling pipeline: base = 'kernel_mul_mm_f32_f32', name = 'kernel_mul_mm_f32_f32_bci=0_bco=0_ne12=1_ne13=1_r2=1_r3=1'
ggml_metal_library_compile_pipeline: loaded kernel_mul_mm_f32_f32_bci=0_bco=0_ne12=1_ne13=1_r2=1_r3=1      0x74d7e4a80 | th_max = 1024 | th_width =   32
  MUL_MAT(type_a=f32,type_b=f32,m=16384,n=32768,k=64,bs=[1,1],nr=[1,1],per=[0,1,2,3],k_v=0,o=1,src_overlap=0): OK
[MUL_MAT] ERR = 0.000982691 > 0.000500000   MUL_MAT(type_a=f32,type_b=f32,m=16384,n=32800,k=64,bs=[1,1],nr=[1,1],per=[0,1,2,3],k_v=0,o=1,src_overlap=0): FAIL
[MUL_MAT] ERR = 0.004202582 > 0.000500000   MUL_MAT(type_a=f32,type_b=f32,m=16384,n=32900,k=64,bs=[1,1],nr=[1,1],per=[0,1,2,3],k_v=0,o=1,src_overlap=0): FAIL
  2/4 tests passed

Failing tests:
  MUL_MAT(type_a=f32,type_b=f32,m=16384,n=32800,k=64,bs=[1,1],nr=[1,1],per=[0,1,2,3],k_v=0,o=1,src_overlap=0)
  MUL_MAT(type_a=f32,type_b=f32,m=16384,n=32900,k=64,bs=[1,1],nr=[1,1],per=[0,1,2,3],k_v=0,o=1,src_overlap=0)
  Backend MTL0: FAIL
ggml_metal_free: deallocating
Backend 2/3: BLAS
  Skipping
Backend 3/3: CPU
  Skipping
2/3 backends passed
FAIL

Result after the change

ggml_metal_device_init: GPU name:   MTL0 (Apple M5)
ggml_metal_device_init: GPU family: MTLGPUFamilyApple10  (1010)
ggml_metal_device_init: GPU family: MTLGPUFamilyCommon3 (3003)
ggml_metal_device_init: GPU family: MTLGPUFamilyMetal4  (5002)
ggml_metal_device_init: simdgroup reduction   = true
ggml_metal_device_init: simdgroup matrix mul. = true
ggml_metal_device_init: has unified memory    = true
ggml_metal_device_init: has bfloat            = true
ggml_metal_device_init: has tensor            = true
ggml_metal_device_init: use residency sets    = true
ggml_metal_device_init: use shared buffers    = true
ggml_metal_device_init: recommendedMaxWorkingSetSize  = 12713.12 MB
Testing 3 devices

ggml_metal_init: allocating
ggml_metal_init: found device: Apple M5
ggml_metal_init: picking default device: Apple M5
ggml_metal_init: use fusion         = true
ggml_metal_init: use concurrency    = true
ggml_metal_init: use graph optimize = true
Backend 1/3: MTL0
  Device description: Apple M5
  Device memory: 12124 MB (12123 MB free)

ggml_metal_library_compile_pipeline: compiling pipeline: base = 'kernel_mul_mm_f32_f32', name = 'kernel_mul_mm_f32_f32_bci=0_bco=1_ne12=1_ne13=1_r2=1_r3=1'
ggml_metal_library_compile_pipeline: loaded kernel_mul_mm_f32_f32_bci=0_bco=1_ne12=1_ne13=1_r2=1_r3=1      0xb4045c000 | th_max = 1024 | th_width =   32
  MUL_MAT(type_a=f32,type_b=f32,m=16384,n=32700,k=64,bs=[1,1],nr=[1,1],per=[0,1,2,3],k_v=0,o=1,src_overlap=0): OK
ggml_metal_library_compile_pipeline: compiling pipeline: base = 'kernel_mul_mm_f32_f32', name = 'kernel_mul_mm_f32_f32_bci=0_bco=0_ne12=1_ne13=1_r2=1_r3=1'
ggml_metal_library_compile_pipeline: loaded kernel_mul_mm_f32_f32_bci=0_bco=0_ne12=1_ne13=1_r2=1_r3=1      0xb4045ca80 | th_max = 1024 | th_width =   32
  MUL_MAT(type_a=f32,type_b=f32,m=16384,n=32768,k=64,bs=[1,1],nr=[1,1],per=[0,1,2,3],k_v=0,o=1,src_overlap=0): OK
  MUL_MAT(type_a=f32,type_b=f32,m=16384,n=32800,k=64,bs=[1,1],nr=[1,1],per=[0,1,2,3],k_v=0,o=1,src_overlap=0): OK
  MUL_MAT(type_a=f32,type_b=f32,m=16384,n=32900,k=64,bs=[1,1],nr=[1,1],per=[0,1,2,3],k_v=0,o=1,src_overlap=0): OK
  4/4 tests passed
  Backend MTL0: OK
ggml_metal_free: deallocating
Backend 2/3: BLAS
  Skipping
Backend 3/3: CPU
  Skipping
3/3 backends passed
OK

@ggerganov
ggerganov merged commit 48e9857 into ggml-org:master Oct 12, 2026
1 check passed
@BrewTestBot BrewTestBot mentioned this pull request Oct 15, 2026
1 task done
edwardyoon pushed a commit to edwardyoon/focus-llama that referenced this pull request Oct 24, 2026
…g#28748)

* metal: fix metal tensor kernel_mul_mm error for large tensor

Assisted-by: Codex

* Update ggml/src/ggml-metal/kernels/mul_mm.metal

Co-authored-by: YiChen Lv <63285796+forforever73@users.noreply.github.com>

---------

Co-authored-by: 向锡铭 <xiangximing@xiangximings-MacBook-Air.local>
Co-authored-by: YiChen Lv <63285796+forforever73@users.noreply.github.com>
auRose94 pushed a commit to auRose94/llama.cpp that referenced this pull request Oct 28, 2026
…g#28748)

* metal: fix metal tensor kernel_mul_mm error for large tensor

Assisted-by: Codex

* Update ggml/src/ggml-metal/kernels/mul_mm.metal

Co-authored-by: YiChen Lv <63285796+forforever73@users.noreply.github.com>

---------

Co-authored-by: 向锡铭 <xiangximing@xiangximings-MacBook-Air.local>
Co-authored-by: YiChen Lv <63285796+forforever73@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Apple Metal https://en.wikipedia.org/wiki/Metal_(API) ggml changes relating to the ggml tensor library for machine learning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants