Skip to content

metal: support non-zero left padding for PAD - #25827

Closed
flyingtimes wants to merge 1 commit into
ggml-org:masterfrom
flyingtimes:metal-pad-left-padding
Closed

metal: support non-zero left padding for PAD#25827
flyingtimes wants to merge 1 commit into
ggml-org:masterfrom
flyingtimes:metal-pad-left-padding

Conversation

@flyingtimes

Copy link
Copy Markdown

Overview

The Metal PAD kernel only handled right padding (all left paddings lp0/lp1/lp2/lp3 == 0). For any PAD node with non-zero left padding, ggml_metal_device_supports_op returned false, silently skipping the op and breaking any model whose graph depends on it.

This was hit in the wild by CosyVoice's flow-matching decoder, which emits PAD(lp0=2, rp0=0); the model could not run on Metal and fell back to CPU (~6x slower on Apple silicon).

Root cause

// ggml-metal-device.m
case GGML_OP_PAD:
    ...
    return (lp0 == 0) && (lp1 == 0) && (lp2 == 0) && (lp3 == 0);

kernel_pad_impl assumed source and destination were aligned at the origin of every dim (i0x = i_x), so it only produced correct results when there was no left padding.

Fix

Compute source coords as i0x = i_x - lp_x, writing zero outside [0, ne0x). This mirrors the CPU reference implementation (ggml_compute_forward_pad_f32).

  • ggml-metal-impl.h: add lp0..lp3 to ggml_metal_kargs_pad
  • ggml-metal.metal: kernel_pad_impl applies the per-dim left offset
  • ggml-metal-ops.cpp: populate lp0..lp3 from op_params
  • ggml-metal-device.m: drop the lp == 0 requirement

Circular padding (op_params[8] != 0) is still not supported and keeps returning false; that is tracked separately by #16985.

Compatibility

With all lp == 0 the kernel is identical to the previous version (all pre-existing right-padding tests continue to pass).

Verification

test-backend-ops test -o PAD on Apple M2 Ultra (Metal vs CPU reference), with new regression cases for left padding:

PAD(ne_a=[101,1024,1,1], lp0=2, rp0=0):           OK   [new]
PAD(ne_a=[512,512,1,1],  lp0=4, rp0=0):           OK   [new]
PAD(ne_a=[512,512,1,1],  lp1=3, rp1=0):           OK   [new]
PAD(ne_a=[64,64,3,1],    lp0=2, rp0=3, lp1=1):    OK   [new, mixed]
... all pre-existing cases still OK
PAD(..., circular=1):                              not supported [unchanged]

End-to-end, CosyVoice3-0.5B now runs on Metal: tts_generate for a 50-char prompt drops from ~14.5s (CPU) to ~2.4s (Metal).

The Metal PAD kernel only handled right padding (all left paddings
lp0/lp1/lp2/lp3 == 0). For any PAD node with non-zero left padding,
ggml_metal_device_supports_op returned false, silently skipping the op
and breaking any model whose graph depends on it.

Hit in the wild by CosyVoice's flow-matching decoder, which emits
PAD(lp0=2, rp0=0); the model could not run on Metal and fell back to
CPU (~6x slower on Apple silicon).

Fix: compute source coords as i0x = i_x - lp_x, writing zero outside
[0, ne0x). Mirrors the CPU reference implementation
(ggml_compute_forward_pad_f32). Circular padding (op_params[8] != 0)
remains unsupported and still returns false; tracked separately by ggml-org#16985.

Backward compatible: with all lp == 0 the kernel is identical to the
previous version.

Verified with test-backend-ops test -o PAD on Apple M2 Ultra (Metal vs
CPU reference), including new left-padding regression cases.
@flyingtimes
flyingtimes requested review from a team and ggerganov as code owners July 17, 2026 13:51
@ggml-gh-bot

ggml-gh-bot Bot commented Jul 17, 2026

Copy link
Copy Markdown

Hi @flyingtimes, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • PR Template not respected: Please respect the template when creating a new pull request. Make sure to fill out all required sections.

Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

@github-actions github-actions Bot added testing Everything test related ggml changes relating to the ggml tensor library for machine learning Apple Metal https://en.wikipedia.org/wiki/Metal_(API) labels Jul 17, 2026
@flyingtimes

Copy link
Copy Markdown
Author

Closing in favor of the upstream-track fix at the consumer level (cosyvoice.cpp maintains a local patch for this in cmake/patches/ggml-metal-pad-beg.patch).

After looking closer I realized this overlaps with work already done in the cosyvoice.cpp tree (originally PR Lourdle/cosyvoice.cpp#2 by @jasagiri), and the Metal PAD left-padding behavior is already tracked by the existing // TODO: add circular padding support for metal comment here. I'll route the fix through the consumer project's patch instead to avoid duplicating effort.

Thanks for the consideration. Happy to reopen if a maintainer feels the upstream change is still wanted.

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 testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant