You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Use the new pattern matching operators (| and optional) to simplify the patterns.
Add transpose_b_scale and transpose_b_zp to track the actual layout of scale/zp.
Current logic:
If weights and scale/zp are transposed, scale/zp will be reordered back to a non-transposed layout to satisfy kernel requirements.
If weights and scale/zp are not transposed, weights and scale/zp will first be reordered to a transposed layout. Since the kernel still requires scale/zp in a non-transposed layout, another reorder is added to convert scale/zp back.
As a result, in case 2, scale/zp are unnecessarily transposed and then transposed back. This increases memory usage and adds extra compilation work. The impact is currently limited because the tensors are constants and the cost is paid only during compilation.
With this PR:
By tracking the actual layout of scale/zp, the plugin can skip the unnecessary transpose-and-revert steps in case 2. This simplifies the logic, reduces memory usage, and will be especially helpful when weights/scale/zp are provided as parameters instead of constants.
Data flow (non-transposed case)
Weights [K,N] --Transpose--> FC (transpose_b=true)
Scale [G,N] -------------> FC (transpose_b_scale=false) // no Transpose, no reorder
ZP [G,N] -------------> FC (transpose_b_zp=false)
OCL : bfyx[G,N] reinterpreted as fbyx (N=batch, G=feature)
oneDNN: ngroups read from axis 0 ([G,N])
This optional scale reshape is matched but then discarded: the callback still builds fc_input_scale from the raw mul_const_m. For a valid 3D decompression such as weights [G,N,K] with scale [G,N] -> Reshape([G,N,1]), the callback sees a rank-2 scale and reaches the constant-weight assertion at convert_fc_to_compressed.cpp:147 instead of leaving or converting the graph. Consume/fold the matched mul_const_reshape_m when constructing the compressed FC, or do not match this form yet.
For non-transposed parameters, rank - 1 is not always the N axis: legacy padded constants can be [groups,N,1,1] (the same representation handled in prepare_quantization.cpp:578-589). This selects axis 3, so FCs with different N sizes fail Concat validation instead of being fused along axis 1. Determine the logical two-dimensional suffix/prefix after accounting for trailing unit padding and leading broadcast dimensions, as the oneDNN helper does.
### Details:
- openvinotoolkit#38697 added handling for an OC-broadcast decompression
scale/zero-point (`[1, G]` / `[G, 1]`) in the oneDNN FullyConnected
implementation: `get_grouped_mask()` clears the OC bit from the grouped
scale/zp mask whenever the OC dim of the scale/zp is 1.
- That check didn't consider whether the weights' N is also 1, in which
case the scale isn't really broadcast. For example, u4 weights `[1,
2048]` with a transposed scale `[1, 16]` (group size 128) lost the OC
bit; for a 2D FC that leaves mask = 1, K only. oneDNN's gemm doesn't
support that mask, so oneDNN falls back to the reference `ocl:ref:any`
matmul, a performance regression for this dynamic-shape FC.
- On re-evaluation, the OC-broadcast handling is overly defensive. The
standard weight-compression flow (NNCF/optimum) always produces per-OC
scale/zp `[N, G, 1]`, which
`ConvertFullyConnectedToFullyConnectedCompressed` reshapes to `[N, G]`,
so the scale/zp OC dim always matches the weights' N.
- This PR removes `get_grouped_mask()` and restores the full `grouped`
mask for grouped scale/zp in both `create()` and `load()`, i.e. the
behavior before openvinotoolkit#38697. It also removes the
`compressed_int8_broadcast_scale_zp` unit tests, which covered only the
removed path.
### Tickets:
- *[CVS-197314](https://jira.devtools.intel.com/browse/CVS-197314)*
### AI Assistance:
- AI assistance used: yes
- Used AI to trace oneDNN matmul implementation selection (`gemm_t` vs
`ref_t`), analyze the scale/zp mask computed by `get_grouped_mask()`,
check where scale/zp shapes come from in the compression pipeline, and
draft this description.
…e treated as grouped case (openvinotoolkit#39268)
## Description
For weights [64, N, K] and zero points [64, N, 1], the old code
incorrectly read N as the group count but accidentally selected a
grouped mask that preserved per-expert zero points. After the PR openvinotoolkit#38697
correctly identified one group along K, the existing rank check selected
a per-channel mask that ignored the expert axis, causing all experts to
reuse the first expert’s zero points. The fix explicitly checks whether
weights have a batch/expert dimension and retains the grouped mask for
3D weights, including during cache loading.
| Stage | IFM-group axis read | `ngroups` | Zero-point mask | Expert
handling |
|---|---|---|---|---|
| Before PR openvinotoolkit#38697 | 1 (incorrect) | N (incorrect) | 7, grouped |
Preserved by accident |
| After PR openvinotoolkit#38697, before fix | 2 (correct) | 1 (correct) | 4,
per-channel | Expert axis ignored |
| After current fix | 2 (correct) | 1 (correct) | 7, grouped | Expert
axis preserved |
## Details
**Affected topology:**
- Weights: `[64, N, K]`, with `transpose_b = true`.
- Zero points: `[64, N, 1]`, with `transpose_b_zp = true`.
- Assume `N > 1` and `K > 1`, so the weights' non-unit dimension count
is 3.
#### Before the PR openvinotoolkit#38697: correct expert handling by accident
The old code chose the zero-point IFM axis using the number of non-unit
zero-point dimensions:
```cpp
// Zero-point shape: [64, N, 1]
dzp_rank = count_if(dims > 1); // 2, because the trailing 1 is ignored
dzp_ifm_dim_idx = weights_transposed
? dzp_rank - 1
: dzp_rank - 2; // 1
ngroups = dzp_layout.get_dim(1); // N, not the actual group count of 1
```
Because `ngroups != 1`, the code selected the grouped branch:
```cpp
attr->set_zero_points(DNNL_ARG_WEIGHTS, grouped, {group_size, 1}, dzp_data_type);
```
For a 3D FC, `grouped = (1 << 3) - 1 = 7`. This mask includes the expert
dimension, so different experts retain their own zero points. The
observed case worked, but the axis selection and inferred group count
were wrong.
#### After the PR openvinotoolkit#38697, before the fix: correct axis exposes the
mask-selection bug
The new orientation-aware helper correctly identifies the IFM-group
axis:
```cpp
get_ifm_dim_idx(dzp_layout, true, 3); // 2
ngroups = dzp_layout.get_dim(2); // 1
```
However, the per-channel condition still used the non-unit zero-point
rank:
```cpp
// dzp_rank is still 2 for [64, N, 1].
if (ngroups == 1 && dzp_rank <= 2) {
attr->set_zero_points(DNNL_ARG_WEIGHTS, per_oc, {}, dzp_data_type);
}
```
It didn't land to the condition which has group attribute.
#### After the fix in current PR
Mask selection now explicitly checks whether the weights have a
batch/expert dimension:
```cpp
const bool has_batch_dim = weight_rank > 2;
if (ngroups == 1 && !has_batch_dim) {
attr->set_zero_points(DNNL_ARG_WEIGHTS, per_oc, {}, dzp_data_type);
} else {
attr->set_zero_points(DNNL_ARG_WEIGHTS, grouped, {group_size, 1}, dzp_data_type);
}
```
For `[64, N, K]`, `has_batch_dim` is `true`. Even with `ngroups == 1`,
the code keeps the grouped mask 7 and preserves per-expert zero points.
Ordinary 2D per-channel weights still use the per-channel branch.
The same batch-dimension check is applied to scale and zero-point
attributes during cache loading, keeping grouped handling consistent
with fresh compilation.
## Ticket
* CVS-198171
## AI Assistance:
* Yes
* Used AI to find the root cause; engineer manually build OV- GenAI to
validate the fix
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
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.
e-ddykim
left a comment
github-actions
Bot
added
category: build
Details:
The reason for this refactor is to make the code cleaner and better prepare for upcoming features:
Changes:
| and optional) to simplify the patterns.transpose_b_scaleandtranspose_b_zpto track the actual layout of scale/zp.Current logic:
If weights and scale/zp are transposed, scale/zp will be reordered back to a non-transposed layout to satisfy kernel requirements.
If weights and scale/zp are not transposed, weights and scale/zp will first be reordered to a transposed layout. Since the kernel still requires scale/zp in a non-transposed layout, another reorder is added to convert scale/zp back.
As a result, in case 2, scale/zp are unnecessarily transposed and then transposed back. This increases memory usage and adds extra compilation work. The impact is currently limited because the tensors are constants and the cost is paid only during compilation.
With this PR:
By tracking the actual layout of scale/zp, the plugin can skip the unnecessary transpose-and-revert steps in case 2. This simplifies the logic, reduces memory usage, and will be especially helpful when weights/scale/zp are provided as parameters instead of constants.
Tickets:
AI Assistance:
AI assistance used to implement fix and unit test. The generated code was verified for correctness, manually built, executed, and fixed as needed.