vulkan int8 kernels#6751
Conversation
Implement Vulkan int8 paths for convolution, convolutiondepthwise, innerproduct, and gemm with scalar fallback and integer dot product shader branches. Add goal.md and enable existing int8 unit tests for Vulkan coverage.
|
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #6751 +/- ##
==========================================
- Coverage 95.78% 94.80% -0.99%
==========================================
Files 946 946
Lines 410528 413891 +3363
==========================================
- Hits 393237 392402 -835
- Misses 17291 21489 +4198 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 034f555987
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (int i = 0; i < psc(w); i += 4) | ||
| { | ||
| const int v4 = i8buffer_sm4(bottom_blob_int8_data, i / 4); |
There was a problem hiding this comment.
When the flattened InnerProduct input length is not a multiple of 4 and the non-GEMM int8 path is used, the flatten/quantize shapes remain elempack1, but this shader still loads each group as a 32-bit sint8vec4. For example a 1-D int8 input with 6 elements reaches the iteration i == 4 and reads bytes 4..7 even though only bytes 4..5 belong to the VkMat; the padded weight lanes being zero does not make the out-of-bounds buffer load valid. Please pack/zero-pad the input to num_input_packed or load the tail lanes with bounded i8buffer_ld1 accesses.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e709465b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| for (int i = start; i < end; i++) | ||
| { | ||
| const int v4 = i8buffer_sm4(bottom_blob_int8_data, i); |
There was a problem hiding this comment.
When the flattened int8 input is large enough to use the sum8 path (num_input_packed / 4 >= 32) but its true length is not divisible by 4, this loop rounds psc(w) up and reads the final input group as a full sint8vec4. The flattened VkMat remains scalar/elempack1 with only the real elements initialized, so a length like 130 makes the last iteration consume bytes 128..131 even though only 128..129 are tensor data; fixing only the similar innerproduct_int8.comp tail case would still leave large non-GEMM InnerProduct inputs reading padding/invalid lanes here. Please either zero-pad/pack the flattened input to num_input_packed or load the last lanes with bounded scalar reads.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2109b5aa92
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| bvec4 mask = bvec4(uvec4(x >= 0 && x < psc(w) && y >= 0 && y < psc(h)) & (uvec4(greaterThanEqual(z4, ivec4(0))) & uvec4(lessThan(z4, ivec4(psc(c)))))); | ||
|
|
||
| ivec4 pad_value4 = per_channel_pad == 1 ? i8buffer_ld4(per_channel_pad_blob_data, gz) : pad_value; |
There was a problem hiding this comment.
When an int8 Padding layer needs the pack1-to-pack4 shader, this line makes shader compilation fail because the ternary operands have different GLSL types: i8buffer_ld4(...) expands to an ivec4, while pad_value is an int. This is checked when the pipeline is compiled even when per_channel_pad is specialized to 0, so any int8 pack1-to-pack4 padding case (for example channel padding that makes the output channel count divisible by 4) cannot run; use an ivec4(pad_value) fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae1231abec
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| const int elempack = opt.use_packing_layout && num_input % 4 == 0 ? 4 : 1; | ||
|
|
||
| if (!bottom_is_int8) |
There was a problem hiding this comment.
When the input is already an 8-bit VkMat, this path skips the quantize helper that would create the num_input % 4 == 0 ? 4 : 1 layout, and it never converts bottom.elempack to the elempack specialization used by the int8 convolution shaders. For an int8 pack1 producer feeding a convolution whose input channel count is divisible by 4, the channel-count check still passes, but convolution_packed_int8.comp takes its elempack == 4 branch and reads the pack1 buffer through the bottom_blob_4 binding. Mirror the depthwise path by converting already-int8 inputs to the expected pack before dispatch.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f11fe82ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| int out_elempack = 1; | ||
| if (use_cooperative_matrix) |
There was a problem hiding this comment.
When int8 Gemm runs the non-cooperative path (for example on devices without int8 cooperative matrix support, or when cooperative matrices are disabled) and the model sets output_elempack to 4, this leaves out_elempack at 1 and allocates a scalar output even though the caller requested pack4. The regular Vulkan Gemm applies output_elempack before allocation, while gemm_int8.comp only writes a scalar sfp buffer, so downstream layers/shapes that expect the requested pack4 layout receive the wrong layout.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12dcacfc4e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| sumfp32 = activation_afpvec4(sumfp32, activation_type, activation_param_0, activation_param_1); | ||
|
|
||
| buffer_st4(top_blob_data, gx / 4, sumfp32); |
There was a problem hiding this comment.
When the non-GEMM int8 InnerProduct path has num_output not divisible by 4 and the sum8 path is not used, forward_int8 allocates top_blob with out_elempack == 1, but this shader still dispatches ceil(num_output/4) groups and writes a full sfpvec4 for the last group. For example num_output == 5 allocates 5 scalar outputs while the second invocation writes lanes 4..7, overrunning or corrupting data beyond the VkMat; the tail needs scalar/bounded stores or the output must be allocated as padded pack4.
Useful? React with 👍 / 👎.
|
|
||
| sumfp32 = activation_afpvec4(sumfp32, activation_type, activation_param_0, activation_param_1); | ||
|
|
||
| buffer_st4(top_blob_data, gx, sumfp32); |
There was a problem hiding this comment.
When the large-input int8 InnerProduct uses the sum8 path and num_output is not divisible by 4, top_blob is still allocated as scalar (out_elempack == 1), but the reducer writes one full sfpvec4 for each rounded output group. A case like num_output == 5 makes the final reducer invocation store four lanes even though only one output remains, so the shader writes past the scalar output buffer; mirror the tail handling here or allocate a padded pack4 output.
Useful? React with 👍 / 👎.
3 participants
No description provided.