From f04148f1f42ab97a9f838d0e292c4d8184c8ca5e Mon Sep 17 00:00:00 2001 From: Tamas Petz Date: Fri, 8 Dec 2023 11:25:39 +0100 Subject: [PATCH 1/2] [test] API tests for saturating_absdiff() Co-authored-by: Denes Tarjan --- test/api/test_saturating_absdiff.cpp | 105 +++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 test/api/test_saturating_absdiff.cpp diff --git a/test/api/test_saturating_absdiff.cpp b/test/api/test_saturating_absdiff.cpp new file mode 100644 index 000000000..0eab54522 --- /dev/null +++ b/test/api/test_saturating_absdiff.cpp @@ -0,0 +1,105 @@ +// SPDX-FileCopyrightText: 2023 Arm Limited and/or its affiliates +// +// SPDX-License-Identifier: Apache-2.0 + +#include +#include + +#include + +#include "framework/operation.h" + +#define INTINSICCV_SATURATING_ABSDIFF(type, suffix) \ + INTINSICCV_API(saturating_absdiff, intrinsiccv_saturating_absdiff_##suffix, \ + type) + +INTINSICCV_SATURATING_ABSDIFF(int8_t, s8); +INTINSICCV_SATURATING_ABSDIFF(uint8_t, u8); +INTINSICCV_SATURATING_ABSDIFF(int16_t, s16); +INTINSICCV_SATURATING_ABSDIFF(uint16_t, u16); +INTINSICCV_SATURATING_ABSDIFF(int32_t, s32); + +template +class SaturatingAbsDiffTest final : public BinaryOperationTest { + /// Expose constructor of base class. + using BinaryOperationTest::BinaryOperationTest; + + protected: + using Elements = typename BinaryOperationTest::Elements; + using BinaryOperationTest::min; + using BinaryOperationTest::max; + + /// Calls the API-under-test in the appropriate way. + void call_api() override { + saturating_absdiff()( + this->inputs_[0].data(), this->inputs_[0].stride(), + this->inputs_[1].data(), this->inputs_[1].stride(), + this->actual_[0].data(), this->actual_[0].stride(), this->width(), + this->height()); + } + + /// Returns different test data for signed and unsigned element types. + const std::vector& test_elements() override { + if constexpr (std::is_unsigned_v) { + static const std::vector kTestElements = { + // clang-format off + { 0, 0, 0}, + { 1, 1, 0}, + { 2, 1, 1}, + { 1, 2, 1}, + { 0, max(), max()}, + {max(), max(), 0}, + {max(), max() - 1, 1}, + {max(), 0, max()}, + // clang-format on + }; + + return kTestElements; + } else { + static const std::vector kTestElements = { + // clang-format off + {min(), max(), max()}, + {max(), min(), max()}, + { -1, max(), max()}, + {max(), -1, max()}, + {min(), min() + 1, 1}, + { -1, -2, 1}, + { -2, -1, 1}, + { -1, -1, 0}, + { 0, 0, 0}, + { 1, 1, 0}, + { 2, 1, 1}, + { 1, 2, 1}, + { 0, max(), max()}, + {max(), max(), 0}, + {max(), max() - 1, 1}, + {max(), 0, max()}, + // clang-format on + }; + + return kTestElements; + } + } +}; // end of class SaturatingAbsDiffTest + +template +class SaturatingAbsDiff : public testing::Test { + public: + /// Dummy value, only used to get the type. + ElementType value_; +}; // end of class SaturatingAbsDiff + +using ElementTypes = + ::testing::Types; +TYPED_TEST_SUITE(SaturatingAbsDiff, ElementTypes); + +/// Tests \ref intrinsiccv_saturating_absdiff_ API. +TYPED_TEST(SaturatingAbsDiff, API) { + using ElementType = decltype(this->value_); + // Test without padding. + SaturatingAbsDiffTest{}.test(); + // Test with padding. + SaturatingAbsDiffTest{} + .with_padding(test::Options::vector_length()) + .test(); +} -- GitLab From 6c69c98d240de7255e08fdb4560115d3179546ef Mon Sep 17 00:00:00 2001 From: Tamas Petz Date: Fri, 8 Dec 2023 13:40:10 +0100 Subject: [PATCH 2/2] [NEON] Fix scalar path of saturating_absdiff() In the previous version 'decltype(src_a - src_b)' was still signed, therefore it chose the first argument even when max() should have been chosen. The new variant calculates unsigned difference and then applies saturating cast to limit the domain. Co-authored-by: Denes Tarjan --- intrinsiccv/include/utils.h | 24 ++++++++++++++++++++ intrinsiccv/src/arithmetics/absdiff_neon.cpp | 13 +++++++---- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/intrinsiccv/include/utils.h b/intrinsiccv/include/utils.h index 56e62e784..74f2f2d11 100644 --- a/intrinsiccv/include/utils.h +++ b/intrinsiccv/include/utils.h @@ -27,6 +27,30 @@ static U saturating_cast(S value) INTRINSICCV_STREAMING_COMPATIBLE { return static_cast(value); } +// Saturating cast from unsigned to unsigned type. +template < + typename SrcType, typename DstType, + std::enable_if_t && std::is_unsigned_v, + bool> = true> +static DstType saturating_cast(SrcType value) INTRINSICCV_STREAMING_COMPATIBLE { + return static_cast(value); +} + +// Saturating cast from unsigned to signed type. +template < + typename SrcType, typename DstType, + std::enable_if_t && std::is_unsigned_v, + bool> = true> +static DstType saturating_cast(SrcType value) INTRINSICCV_STREAMING_COMPATIBLE { + DstType max_value = std::numeric_limits::max(); + + if (value > static_cast(max_value)) { + return max_value; + } + + return static_cast(value); +} + // Rounding shift right. template static T rounding_shift_right(T value, diff --git a/intrinsiccv/src/arithmetics/absdiff_neon.cpp b/intrinsiccv/src/arithmetics/absdiff_neon.cpp index 496fede2b..856605f35 100644 --- a/intrinsiccv/src/arithmetics/absdiff_neon.cpp +++ b/intrinsiccv/src/arithmetics/absdiff_neon.cpp @@ -2,8 +2,7 @@ // // SPDX-License-Identifier: Apache-2.0 -#include -#include +#include #include "intrinsiccv.h" #include "neon.h" @@ -26,9 +25,13 @@ class SaturatingAbsDiff final : public UnrollTwice { } ScalarType scalar_path(ScalarType src_a, ScalarType src_b) { - return std::min( - src_a > src_b ? src_a - src_b : src_b - src_a, - std::numeric_limits::max()); + using UnsignedScalarType = std::make_unsigned_t; + // Calculate unsigned difference and then apply saturating cast. + UnsignedScalarType u_src_a = static_cast(src_a); + UnsignedScalarType u_src_b = static_cast(src_b); + UnsignedScalarType difference = + src_a > src_b ? u_src_a - u_src_b : u_src_b - u_src_a; + return saturating_cast(difference); } }; // end of class SaturatingAbsDiff -- GitLab