Skip to content

feat: enable QASYMM8_SIGNED to F32 assembly dequantization#1302

Open
morgolock wants to merge 1 commit into
mainfrom
pr/asm_dequant_f32
Open

feat: enable QASYMM8_SIGNED to F32 assembly dequantization#1302
morgolock wants to merge 1 commit into
mainfrom
pr/asm_dequant_f32

Conversation

@morgolock

Copy link
Copy Markdown
Contributor

Enable QASYMM8_SIGNED input and weights to use the F32 DequantizeFloat assembly output stage, including the direct convolution selection path.

Propagate input and weight zero-points plus the unrounded mathematical K depth so asymmetric offset correction uses the real GEMM/convolution depth rather than arm_gemm's padded internal K.

Add NEON validation coverage for the direct I8S8F32 convolution path.

Performance was checked on a A76, pinned to CPU 4 with one thread. The change is neutral on large workloads and improves the small NHWC signed int8 to F32 convolution path: QASYMM8_SIGNED/RunSmallDequantizeF32 NHWC/no-activation cases show a 1.36x geomean speedup over github/main, with the tiniest single-batch cases improving by roughly 1.45x to 3.13x.

Change-Id: Ie723d3da629d48de6de737c425bf7ad48e0f7feb

@morgolock
morgolock force-pushed the pr/asm_dequant_f32 branch 5 times, most recently from 5aa921d to 002c426 Compare July 2, 2026 09:50
Comment thread src/cpu/operators/CpuGemmDirectConv2d.h Outdated
Comment thread src/cpu/operators/internal/CpuGemmAssemblyDispatch.cpp Outdated
Comment thread src/cpu/operators/internal/CpuGemmAssemblyDispatch.h
Comment thread tests/validation/fixtures/ConvolutionLayerFixture.h Outdated
Comment thread tests/validation/NEON/ConvolutionLayer.cpp Outdated
Comment thread tests/validation/NEON/ConvolutionLayer.cpp Outdated
Comment thread tests/validation/NEON/ConvolutionLayer.cpp Outdated
Comment thread tests/validation/NEON/ConvolutionLayer.cpp Outdated
@Passavee-Losripat

Copy link
Copy Markdown

Hi, I've been testing this PR for OpenVINO's ARM CPU plugin int8 enablement and found a possible validation gap in the new QASYMM8_SIGNED->F32 dequantization path.

QSYMM8_PER_CHANNEL weights are correctly rejected by validate() on this path. However, QASYMM8_SIGNED weights whose QuantizationInfo carries a per-channel scale vector are accepted and the kernel then dequantizes every output channel with scale[0] (the scalar uniform().scale product in create_arm_gemm_dequant).

I create minimal repo to test this: all-ones i8 input, all-ones 3×3×3 i8 weights, bias 0, distinct per-channel scales s[c] -> each interior output should be 27·s[c]:

channel    got      expected(27*s[c])    ratio
c00      0.0270        0.0270            1.0000
c01      0.0270        0.0540            0.5000
c02      0.0270        0.1080            0.2500
...
c11      0.0270       55.2960            0.0005

validate() returns OK, but every channel is computed with s[0]. Since quantization frameworks (NNCF, PyTorch, TF) produce per-channel weight scales by default, this input shape is what integrations like OpenVINO naturally pass, so it fails silently on real models rather than erroring out.

I am thinking of rejecting scale().size() > 1 on this path the same way QSYMM8_PER_CHANNEL is rejected, unless per-channel support is planned. I can share the full standalone test program if useful.

Comment thread src/cpu/operators/internal/CpuGemmAssemblyDispatch.cpp Outdated
* |:--------------|:--------------|:--------------|:--------------|
* |QASYMM8 |QASYMM8 |S32 |QASYMM8 |
* |QASYMM8_SIGNED |QASYMM8_SIGNED |S32 |QASYMM8_SIGNED |
* |QASYMM8_SIGNED |QASYMM8_SIGNED |F32 |F32 |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about Fp16? And, Do we support mixed sign inputs? Such as UInt8 inputs and Int8 weights. That's what I felt from the changes in the glue layer. I also don't see the tests for mixed sign inputs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. For CpuGemmDirectConv2d, this PR only exposes QASYMM8_SIGNED input + QASYMM8_SIGNED weights + F32 bias producing F32 output.

I removed the mixed-sign uint8_t + int8_t -> F32 DequantizeFloat assembly selector/configure path so the glue layer no longer implies direct mixed-sign support, and added validation coverage showing the direct-conv path rejects both FP16 output and mixed QASYMM8 input with QASYMM8_SIGNED weights. The lower-level signed GEMMLowp FP16 path remains intact because it is separately covered by existing GEMMLowp validation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding Fp16 with regular Int8 inputs, I can see it delegates the validation to arm_gemm here:

https://github.com/ARM-software/ComputeLibrary/pull/1302/changes#diff-83002887a0d8a4d2a4abe6c0ddb3c2133695a3dfbe00ca06ffb1b75c746fdf16R1031

I'm on board your with mixed-sign comment. We don't support it as it seems.

{TensorInfo(TensorShape(8U, 6U, 6U), 1, DataType::F32, DataLayout::NHWC),
TensorInfo(TensorShape(16U, 4U, 4U), 1, DataType::F32, DataLayout::NHWC),
TensorInfo(TensorShape(8U, 6U, 6U), 1, DataType::F32, DataLayout::NHWC),
TensorInfo(TensorShape(8U, 6U, 6U), 1, DataType::F32, DataLayout::NHWC)}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we make some F16 & adjust the expected in the body with cpu_supports_dtypes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added an explicit F16 bias/output validation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we end up supporting Fp16 based on the above discussions, we need to add a true case.

Comment thread tests/validation/NEON/ConvolutionLayer.cpp Outdated
Comment thread tests/validation/NEON/ConvolutionLayer.cpp Outdated
Comment thread tests/validation/NEON/ConvolutionLayer.cpp Outdated
@morgolock

morgolock commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Hi, I've been testing this PR for OpenVINO's ARM CPU plugin int8 enablement and found a possible validation gap in the new QASYMM8_SIGNED->F32 dequantization path.

QSYMM8_PER_CHANNEL weights are correctly rejected by validate() on this path. However, QASYMM8_SIGNED weights whose QuantizationInfo carries a per-channel scale vector are accepted and the kernel then dequantizes every output channel with scale[0] (the scalar uniform().scale product in create_arm_gemm_dequant).

I create minimal repo to test this: all-ones i8 input, all-ones 3×3×3 i8 weights, bias 0, distinct per-channel scales s[c] -> each interior output should be 27·s[c]:

channel    got      expected(27*s[c])    ratio
c00      0.0270        0.0270            1.0000
c01      0.0270        0.0540            0.5000
c02      0.0270        0.1080            0.2500
...
c11      0.0270       55.2960            0.0005

validate() returns OK, but every channel is computed with s[0]. Since quantization frameworks (NNCF, PyTorch, TF) produce per-channel weight scales by default, this input shape is what integrations like OpenVINO naturally pass, so it fails silently on real models rather than erroring out.

I am thinking of rejecting scale().size() > 1 on this path the same way QSYMM8_PER_CHANNEL is rejected, unless per-channel support is planned. I can share the full standalone test program if useful.

Hi @Passavee-Losripat

Good catch. I added validation to reject QASYMM8_SIGNED weights with multiple scales on this dequantized path, since the current output stage only supports a single scalar scale.

Please try the latest patchset and let us know if this works for you.

Hope this helps

Enable QASYMM8_SIGNED input and weights to use the F32 DequantizeFloat assembly output stage, including the direct convolution selection path.

Propagate input and weight zero-points plus the unrounded mathematical K depth so asymmetric offset correction uses the real GEMM/convolution depth rather than arm_gemm's padded internal K.

Fix the interleaved no-merge DequantizeFloat scheduler stride so kernels that do not pack row-sum slots advance between A panels correctly.

Guard the symmetric no-merge dequant support helper with the SME/SME2 feature macros. The helper is only referenced when those no-merge dequantized kernels are compiled, so non-SME builds must not define it unconditionally under Werror.

Add NEON validation coverage for the direct I8S8F32 convolution path.

Performance was checked on a Cortex-A76, pinned to CPU 4 with one thread. The change is neutral on large workloads and improves the small NHWC signed int8 to F32 convolution path: QASYMM8_SIGNED/RunSmallDequantizeF32 NHWC/no-activation cases show a 1.36x geomean speedup over github/main, with the tiniest single-batch cases improving by roughly 1.45x to 3.13x.

Validate GEMM3D convolution probes with the real weights and output tensor metadata instead of reusing the input data type for every dummy tensor. This prevents QASYMM8 input with QSYMM8_PER_CHANNEL weights from selecting a 3D path that the real lowp operation cannot safely configure.

When QASYMM8 output is handled through the signed lowp assembly path, route fused assembly through the signed intermediate output, keep the adjusted output stage in sync for configure/validate/runtime quantization updates, and convert back to QASYMM8 after the fused assembly path as well as the fallback path.

Reject non-uniform QASYMM8_SIGNED dequantization metadata: this path only supports one weight scale. Although QSYMM8_PER_CHANNEL is the usual per-channel weights data type, ACL does not enforce uniform quantization from the QASYMM8_SIGNED data type alone; a TensorInfo can still contain a QuantizationInfo scale vector. DequantizeFloat currently consumes only a single scalar scale via QuantizationInfo::uniform(), so accepting a scale vector would silently use scale[0] for every output channel. Reject QASYMM8_SIGNED weights with more than one scale on dequantized F32/F16 convolution and GEMM paths until per-channel DequantizeFloat support is implemented.

Signed-off-by: Pablo Marquez Tello <pablo.tello@arm.com>

Change-Id: I945c24e5cd3d21d857b68de90d27aa18a31f7547
@morgolock
morgolock force-pushed the pr/asm_dequant_f32 branch from 9b59ee9 to a51459b Compare July 22, 2026 14:19
@morgolock
morgolock requested a review from gunes-arm July 22, 2026 14:20
ARM_COMPUTE_RETURN_ERROR_ON_MSG(
input->data_type() == DataType::QASYMM8_SIGNED && weights->data_type() == DataType::QASYMM8_SIGNED &&
output->data_type() == DataType::F32 && weights->quantization_info().scale().size() > 1,
"Per-channel QASYMM8_SIGNED weight scales are not supported for dequantized F32 convolution");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite confused here. When we have per channel quantized weights, the data type should be provided as QSYMM8_PER_CHANNEL. This should be the correct usage assumption throughout the library and how we validate against. We may need to think about trying to prevent this misuse in another way.

@Passavee-Losripat would you mind sharing your reproducible? And, do we have any workloads/integration points that OpenVino relies on ACL accepting a per-channel quantized weights tensor with QASYMM8_SIGNED/QASYMM8? Because, to me, we might prevent some TensorInfo being constructed this way, as it's not the correct usage of the types, but that's going to cause crashes instead of silently validating inside a function/operator.

{})),
"We could not find an optimized kernel for S8/QASYMM8_SIGNED input and S32 output");
}
else if (a->data_type() == DataType::QASYMM8_SIGNED && d->data_type() == DataType::F32)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a bit of a weird situation here.

I'd remove this check because that's what the switch/case is for and remove the S8 case from the switch. Would we breaking anything? I'm not too sure. Only if there is some misuse of data types. Same situation goes with comments and U8 type.

So, I'd suggest

  • Ignore S8 was in the case statement above and just remove a->data_type() == DataType::QASYMM8_SIGNED from the ifs here as it's already checked.
  • Create a ticket to resolve the S8/U8 confusion with tests involved.

* |:--------------|:--------------|:--------------|:--------------|
* |QASYMM8 |QASYMM8 |S32 |QASYMM8 |
* |QASYMM8_SIGNED |QASYMM8_SIGNED |S32 |QASYMM8_SIGNED |
* |QASYMM8_SIGNED |QASYMM8_SIGNED |F32 |F32 |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding Fp16 with regular Int8 inputs, I can see it delegates the validation to arm_gemm here:

https://github.com/ARM-software/ComputeLibrary/pull/1302/changes#diff-83002887a0d8a4d2a4abe6c0ddb3c2133695a3dfbe00ca06ffb1b75c746fdf16R1031

I'm on board your with mixed-sign comment. We don't support it as it seems.

{TensorInfo(TensorShape(8U, 6U, 6U), 1, DataType::F32, DataLayout::NHWC),
TensorInfo(TensorShape(16U, 4U, 4U), 1, DataType::F32, DataLayout::NHWC),
TensorInfo(TensorShape(8U, 6U, 6U), 1, DataType::F32, DataLayout::NHWC),
TensorInfo(TensorShape(8U, 6U, 6U), 1, DataType::F32, DataLayout::NHWC)}),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we end up supporting Fp16 based on the above discussions, we need to add a true case.

&src_info.clone()->set_is_resizable(true), &wgt_info.clone()->set_is_resizable(true),
&dst_info.clone()->set_is_resizable(true), PadStrideInfo(1, 1, 0, 0), WeightsInfo(), Size2D(1U, 1U),
ActivationLayerInfo(), false /*fast_math*/);
const bool expected_direct_conv = data_type == DataType::F32 && cpu_supports_dtypes({data_type});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of unnecessary to check cpu support here as it's fp32 only based on the first check

}
}

TEST_CASE(ValidateRejectsPerChannelWeightScales, framework::DatasetMode::ALL)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to revisit this based on how the per channel execution should be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants