Skip to content

fix: Enable asm requantized max pooling with differing qinfo #1285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
morgolock wants to merge 1 commit into
base: main
Choose a base branch
from
+89 −11
Open

fix: Enable asm requantized max pooling with differing qinfo #1285

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions src/cpu/kernels/internal/CpuPool2dAssemblyWrapperKernel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,35 +134,42 @@ CpuPool2dAssemblyWrapperKernel::validate(const ITensorInfo *src, const ITensorIn

if (src_qinfo != dst_qinfo)
{
ARM_COMPUTE_RETURN_ERROR_ON_MSG(info.pool_type != PoolingType::MAX,
Comment thread
gunes-arm marked this conversation as resolved.
"Assembly kernels only support differing src/dst quantization info for "
"MAX pooling");
ARM_COMPUTE_RETURN_ERROR_ON_MSG(src->data_type() == DataType::QASYMM8_SIGNED &&
(src_qinfo.offset != 0 || dst_qinfo.offset != 0),
"Assembly kernels only support differing src/dst quantization info for "
"QASYMM8_SIGNED when both offsets are zero");
const float multiplier = src_qinfo.scale / dst_qinfo.scale;
int32_t dst_multiplier{};
int32_t dst_shift{};
ARM_COMPUTE_RETURN_ERROR_ON(
quantization::calculate_quantized_multiplier(multiplier, &dst_multiplier, &dst_shift));
!quantization::calculate_quantized_multiplier(multiplier, &dst_multiplier, &dst_shift));
}
else
{
if (src->data_type() == DataType::QASYMM8)
if (src->data_type() == DataType::QASYMM8 || src->data_type() == DataType::QASYMM8_SIGNED)
{
const bool has_padding = info.pad_stride_info.has_padding();
ARM_COMPUTE_RETURN_ERROR_ON_MSG(info.pool_type != PoolingType::MAX && !info.exclude_padding &&
has_padding,
"Assembly kernels only support padded MAX pooling for QASYMM8 with "
"same src/dst quantization info");
"Assembly kernels only support padded MAX pooling for QASYMM8 and "
"QASYMM8_SIGNED with same src/dst quantization info");
}
}
}
else
{
const TensorInfo out_info(dst_shape, 1, src->data_type()); // use src dtype as they're same.
ARM_COMPUTE_RETURN_ERROR_ON_SIZE_UNSUPPORTED(&out_info);
if (src->data_type() == DataType::QASYMM8)
if (src->data_type() == DataType::QASYMM8 || src->data_type() == DataType::QASYMM8_SIGNED)
{
// If dst is not configured, the quantization info are the same
const bool has_padding = info.pad_stride_info.has_padding();
ARM_COMPUTE_RETURN_ERROR_ON_MSG(info.pool_type != PoolingType::MAX && !info.exclude_padding && has_padding,
"Assembly kernels only support padded MAX pooling for QASYMM8 with "
"same src/dst quantization info");
"Assembly kernels only support padded MAX pooling for QASYMM8 and "
"QASYMM8_SIGNED with same src/dst quantization info");
}
}
return Status{};
Expand Down Expand Up @@ -303,9 +310,10 @@ void CpuPool2dAssemblyWrapperKernel::create_arm_pooling_requant(const ITensorInf
int32_t dst_shift{};
quantization::calculate_quantized_multiplier(multiplier, &dst_multiplier, &dst_shift);

const arm_conv::pooling::Requantize32 requant_args(src_qinfo.offset, dst_qinfo.offset,
dst_shift, // left shift
0, // right shift
const int32_t left_shift = std::max(-dst_shift, static_cast<int32_t>(0));
const int32_t right_shift = std::min(-dst_shift, static_cast<int32_t>(0));

const arm_conv::pooling::Requantize32 requant_args(src_qinfo.offset, dst_qinfo.offset, left_shift, right_shift,
dst_multiplier);

// Configure assembly pooling kernel with requantization
Expand Down
72 changes: 71 additions & 1 deletion tests/validation/NEON/PoolingLayer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,28 @@ const auto PoolingDatasetQASYMM8PaddedMax = combine(make("Shape", {TensorShape(7
make("InputQuantInfo", {QuantizationInfo(0.25f, 11)}),
make("OutputQuantInfo", {QuantizationInfo(0.25f, 11)}));

const auto PoolingDatasetQASYMM8SignedPaddedMaxDifferentQInfoZeroOffset =
combine(make("Shape", {TensorShape(7U, 5U, 3U), TensorShape(8U, 7U, 5U)}),
make("PoolingType", {PoolingType::MAX}),
make("PoolingSize", {Size2D(3, 3)}),
make("PadStride", {PadStrideInfo(2, 2, 1, 1)}),
make("ExcludePadding", {false}),
make("DataType", DataType::QASYMM8_SIGNED),
make("DataLayout", {DataLayout::NHWC}),
make("InputQuantInfo", {QuantizationInfo(0.25f, 0)}),
make("OutputQuantInfo", {QuantizationInfo(0.5f, 0)}));

const auto PoolingDatasetQASYMM8SignedPaddedMaxSameQInfo =
combine(make("Shape", {TensorShape(7U, 5U, 3U), TensorShape(8U, 7U, 5U)}),
make("PoolingType", {PoolingType::MAX}),
make("PoolingSize", {Size2D(3, 3)}),
make("PadStride", {PadStrideInfo(2, 2, 1, 1)}),
make("ExcludePadding", {false}),
make("DataType", DataType::QASYMM8_SIGNED),
make("DataLayout", {DataLayout::NHWC}),
make("InputQuantInfo", {QuantizationInfo(0.25f, -11)}),
make("OutputQuantInfo", {QuantizationInfo(0.25f, -11)}));

TEST_SUITE(NEON)
TEST_SUITE(PoolingLayer)

Expand Down Expand Up @@ -163,12 +185,44 @@ DATA_TEST_CASE(Validate, framework::DatasetMode::ALL, zip(
),
input_info, output_info, pool_info, expected)
{
bool is_valid = bool(NEPoolingLayer::validate(&input_info.clone()->set_is_resizable(false), &output_info.clone()->set_is_resizable(false), pool_info));
bool is_valid = bool(NEPoolingLayer::validate(&input_info.clone()->set_is_resizable(false),
&output_info.clone()->set_is_resizable(false),
pool_info));
ARM_COMPUTE_EXPECT(is_valid == expected, framework::LogLevel::ERRORS);
Comment thread
gunes-arm marked this conversation as resolved.
}
// clang-format on
// *INDENT-ON*

TEST_CASE(ValidatePaddedMaxDifferentQuantizationInfo, framework::DatasetMode::ALL)
{
TensorInfo input_info(TensorShape(3U, 15U, 11U, 1U), 1, DataType::QASYMM8, DataLayout::NHWC);
TensorInfo output_info(TensorShape(3U, 8U, 6U, 1U), 1, DataType::QASYMM8, DataLayout::NHWC);
const auto pool_info = PoolingLayerInfo(PoolingType::MAX, 3, DataLayout::NHWC, PadStrideInfo(2, 2, 1, 1), false);

input_info.set_quantization_info(QuantizationInfo(0.25f, 11));
output_info.set_quantization_info(QuantizationInfo(0.5f, 7));

const bool is_valid = bool(NEPoolingLayer::validate(&input_info.clone()->set_is_resizable(false),
&output_info.clone()->set_is_resizable(false), pool_info));

ARM_COMPUTE_EXPECT(is_valid, framework::LogLevel::ERRORS);
}

TEST_CASE(ValidatePaddedAvgDifferentQuantizationInfo, framework::DatasetMode::ALL)
{
TensorInfo input_info(TensorShape(3U, 15U, 11U, 1U), 1, DataType::QASYMM8, DataLayout::NHWC);
TensorInfo output_info(TensorShape(3U, 8U, 6U, 1U), 1, DataType::QASYMM8, DataLayout::NHWC);
const auto pool_info = PoolingLayerInfo(PoolingType::AVG, 3, DataLayout::NHWC, PadStrideInfo(2, 2, 1, 1), false);

input_info.set_quantization_info(QuantizationInfo(0.25f, 11));
output_info.set_quantization_info(QuantizationInfo(0.5f, 7));

const bool is_valid = bool(NEPoolingLayer::validate(&input_info.clone()->set_is_resizable(false),
Copy link
Copy Markdown
Contributor

@gunes-arm gunes-arm Apr 28, 2026

Choose a reason for hiding this comment

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

I don't think you need to clone here. It was for constant objects as far as I recall.

&output_info.clone()->set_is_resizable(false), pool_info));

ARM_COMPUTE_EXPECT(!is_valid, framework::LogLevel::ERRORS);
}

template <typename T>
using NEPoolingLayerIndicesFixture = PoolingLayerIndicesValidationFixture<Tensor, Accessor, NEPoolingLayer, T>;

Expand Down Expand Up @@ -448,6 +502,22 @@ FIXTURE_DATA_TEST_CASE(RunMixedDataLayout,
}
TEST_SUITE_END() // QASYMM8
TEST_SUITE(QASYMM8_SIGNED)
FIXTURE_DATA_TEST_CASE(PaddedMaxSameQInfo,
Copy link
Copy Markdown
Contributor

@gunes-arm gunes-arm Apr 28, 2026

Choose a reason for hiding this comment

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

Can you add QASYMM8 tests, too? They use different underlying kernels. Probably, we need to put arbitrary quantization infos to src and dst as they don't have to be 0 offset.

NEPoolingLayerQuantizedFixture<int8_t>,
framework::DatasetMode::PRECOMMIT,
PoolingDatasetQASYMM8SignedPaddedMaxSameQInfo)
{
// Validate output
validate(Accessor(_target), _reference, tolerance_qasymm8_s);
}
FIXTURE_DATA_TEST_CASE(PaddedMaxDifferentQInfoZeroOffset,
NEPoolingLayerQuantizedFixture<int8_t>,
framework::DatasetMode::PRECOMMIT,
PoolingDatasetQASYMM8SignedPaddedMaxDifferentQInfoZeroOffset)
{
// Validate output
validate(Accessor(_target), _reference, tolerance_qasymm8_s);
}
FIXTURE_DATA_TEST_CASE(RunSmall,
NEPoolingLayerQuantizedFixture<int8_t>,
framework::DatasetMode::PRECOMMIT,
Expand Down