Skip to content

Vulkan: take im2col when the gather is cheap, not only when c_out is large - #22782

Open
msluszniak wants to merge 1 commit into
pytorch:mainfrom
msluszniak:ms/vulkan-im2col-mali-threshold
Open

Vulkan: take im2col when the gather is cheap, not only when c_out is large#22782
msluszniak wants to merge 1 commit into
pytorch:mainfrom
msluszniak:ms/vulkan-im2col-mali-threshold

Conversation

@msluszniak

@msluszniak msluszniak commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Updated twice; this is the third rule. Both earlier versions regressed a model, so the history is worth stating:

  • A c_out gate regressed fast_sam on Mali by 18% (GPU 345 -> 405 ms). Its 80x80 convolutions have c_out 64, so the gate pushed them onto the direct shader.
  • Bounding the gather size regressed fcn_resnet50 by 19% (2938 -> 3505 ms). Its head is a 2048->512 3x3 at 65x65, which wants a 77.9M element gather but reads it back 512 times, so it is exactly the case im2col exists for.

Neither variable works alone because they are independent reasons to take im2col. A large gather is fine when c_out is large, since the GEMM amortizes it c_out times. When c_out is small it is only worth paying if the matrix is small outright, which is where the direct shader on Mali loses badly.

So this keeps upstream's c_out rule and adds a cheap-gather disjunct, scoped to Mali so no other device changes behaviour.

Mali-G76 (Galaxy S10+), fp16, interleaved GPU timestamps, median. "before" is upstream today:

model before c_out gate gather bound this
style_transfer_candy @640 1912.15 1436.31 1410.94 1411.67
fast_sam s @640 345.09 404.54 n/a 341.61
fcn_resnet50 @520 2938.13 2993.47 3505.06 2948.10

fcn gives up 0.3%, at the edge of the run-to-run range. The other two are wins, and neither earlier rule was a win on all three.

Regression check (these runners also carry #22784, which is why the mobilenet models move):

model before this
lraspp @520 71.28 67.78
deeplabv3-mnv3 @520 165.27 161.76
selfie @256 6.49 6.18
ssdlite320 37.79 35.80

32M is calibrated on this device, not derived. The cases it separates are far apart (~4M and ~30M on one side, ~100M and up on the other), so the exact value is not delicate, but a maintainer with other Mali parts may want to revisit it. Adreno is deliberately untouched.

@msluszniak
msluszniak requested a review from SS-JIA as a code owner September 13, 2026 12:09
@pytorch-bot pytorch-bot Bot added the module: vulkan Issues related to the Vulkan delegate and code under backends/vulkan/ label Sep 13, 2026
@pytorch-bot

pytorch-bot Bot commented Sep 13, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22782

Note: Links to docs will display an error until the docs builds have been completed.

⚠️ 16 Awaiting Approval

As of commit 90f63a4 with merge base 903696a (image):

AWAITING APPROVAL - The following workflows need approval before CI can run:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 13, 2026
@github-actions

Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@msluszniak msluszniak changed the title Vulkan: apply the im2col C_out threshold on Mali too Vulkan: bound the conv2d im2col gather Sep 13, 2026
@msluszniak
msluszniak force-pushed the ms/vulkan-im2col-mali-threshold branch from 520b359 to 2baedd2 Compare September 13, 2026 14:15
@msluszniak msluszniak changed the title Vulkan: bound the conv2d im2col gather Vulkan: take im2col when the gather is cheap, not only when c_out is large Sep 13, 2026
Third iteration of this PR. The c_out gate regressed fast_sam 18%, and bounding
the gather regressed fcn_resnet50 by 19% (2938 -> 3505 ms): fcn's head is a
2048->512 3x3 at 65x65, which wants a 77.9M element gather but reads it back
512 times, so it is exactly the case im2col exists for. Neither variable works
alone.

They are independent reasons. A large gather is fine when c_out is large,
because the GEMM amortizes it c_out times. When c_out is small it is only worth
paying if the matrix is small outright, which is where the direct shader on
Mali loses badly. So keep upstream's c_out rule and add a cheap-gather disjunct,
scoped to Mali so no other device changes behaviour.

Measured on a Mali-G76 (Galaxy S10+), fp16, interleaved GPU timestamps, median.
'before' is upstream today:

| model | before | c_out gate | gather bound | this |
| --- | --- | --- | --- | --- |
| style_transfer_candy @640 | 1912.15 | 1436.31 | 1410.94 | **1411.67** |
| fast_sam s @640 | 345.09 | 404.54 | n/a | **341.61** |
| fcn_resnet50 @520 | 2938.13 | 2993.47 | 3505.06 | **2948.10** |

fcn gives up 0.3%, which is at the edge of the run-to-run range. The other two
are wins and neither of the earlier rules was a win on all three.

Regression check (these runners also carry the 5x5 depthwise tile fix, which is
why the mobilenet models move):

| model | before | this |
| --- | --- | --- |
| lraspp @520 | 71.28 | 67.78 |
| deeplabv3-mnv3 @520 | 165.27 | 161.76 |
| selfie @256 | 6.49 | 6.18 |
| ssdlite320 | 37.79 | 35.80 |
@msluszniak
msluszniak force-pushed the ms/vulkan-im2col-mali-threshold branch from 2baedd2 to 90f63a4 Compare September 13, 2026 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: vulkan Issues related to the Vulkan delegate and code under backends/vulkan/

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants