Vulkan: classify conv2d method by weight shape in conv2d_local_wg_size - #22051
Open
msluszniak wants to merge 1 commit into
Open
Vulkan: classify conv2d method by weight shape in conv2d_local_wg_size#22051msluszniak wants to merge 1 commit into
msluszniak wants to merge 1 commit into
Conversation
conv2d_local_wg_size() picked the convolution method from the shader name
alone, and the condition it used matched every conv2d shader:
if (kernel_name.find("conv2d_pw") != npos ||
(kernel_name.find("conv2d") != npos &&
kernel_name.find("conv_transpose2d") == npos)) {
method = Conv2dMethod::Pointwise;
} else {
method = Conv2dMethod::SlidingWindow;
}
The sliding window shader is itself named "conv2d", so it matched and was
labelled Pointwise, which made the SlidingWindow branch unreachable for
every conv2d variant. Only conv_transpose2d reached the else.
The sibling conv2d_global_wg_size() directly above uses the identical
outer name test but then disambiguates by inspecting the weight's spatial
extent, so the two functions could disagree about the same dispatch: the
global size computed as sliding window while the local size was computed
as pointwise.
That second step arrived with pytorch#13173, which introduced the tuned
{64 / y, y, 1} local size for pointwise convolutions. Before it, conv2d
used create_local_wg_size() for every method. The name test swept sliding
window convolutions into the new pointwise size along with it, so this
restores what they had before that commit.
Factors the classification into one function used by both, so they cannot
drift apart again. Depthwise is routed to conv2d_dw_impl() before this
dispatch and never reaches either function, and transposed convolutions
resolve to the same branch as before, so sliding window is the only
behavior that changes.
Both local sizes are always 64 threads per group; only the group shape
differs, for instance {8, 8, 1} to {8, 4, 2} for a 128x128x64 output.
Fixes pytorch#21942
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22051
Note: Links to docs will display an error until the docs builds have been completed.
|
This PR needs a
|
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.
Summary
Fixes #21942.
conv2d_local_wg_size()picked the convolution method from the shader name alone, and the condition it used matched every conv2d shader:The sliding window shader is itself named
conv2d, so it matched and was labelledPointwise, making theSlidingWindowbranch unreachable for every conv2d variant. Onlyconv_transpose2dreached theelse.The sibling
conv2d_global_wg_size()directly above uses the identical outer name test but then disambiguates by inspecting the weight's spatial extent, so the two could disagree about the same dispatch: the global size computed as sliding window while the local size was computed as pointwise. This factors that classification into one function used by both, so they cannot drift apart again.Why this is a restoration, not a new heuristic
The weight-shape check arrived with #13173, which introduced the tuned
{64 / y, y, 1}local size for pointwise convolutions. Before that commit the dispatch usedcreate_local_wg_size(global_size)for every method:The name test swept sliding window convolutions into the new pointwise size along with the pointwise ones, so this restores the local size they had before #13173.
Scope of the behavior change
Only sliding window conv2d changes.
conv2d_dw_impl()before this dispatch and never reaches either function.conv_transpose2ddoes not contain the substringconv2d, so it already fell through tocreate_local_wg_size).create_local_wg_size()is pure arithmetic on the global workgroup size, so the exact before/after can be computed on the host. Across 7452 realistic non-pointwise conv2d output shapes (spatial extents 1 to 512, 3 to 2048 output channels):{8, 8, 1}to one of{8, 4, 2},{4, 8, 2},{2, 8, 4},{4, 2, 8}For example, a 128x128x64 output has global size
{128, 128, 16}and goes from{8, 8, 1}to{8, 4, 2}.Since sliding window convolutions were last benchmarked under
create_local_wg_sizebefore #13173, and the pointwise tuning in that commit was not aimed at them, I would expect this to be neutral to positive. I do not have perf numbers across GPUs, so if you would like this validated on a specific device before landing, say which and I will run it.Test plan
vulkan_backendbuilds clean. The change is a refactor of method classification plus the restored branch; existing conv2d correctness coverage applies unchanged, since the local workgroup size affects scheduling and not results.cc @SS-JIA @manuelcandales @digantdesai @cbilgin