From c2c0d0207df1a1396a1567c4c285dd8a7c925ae9 Mon Sep 17 00:00:00 2001
From: Mark Horvath <mark.horvath@arm.com>
Date: Fri, 2 Aug 2024 10:50:20 +0200
Subject: [PATCH] Change the test kernel for SeparableFilter2D

* Separate values used for kernel_x and kernel_y, and the mask needed
  for test::KernelTest is generated at runtime.
* test::KernelTest is updated to do saturation when calculating expected
  values.
* test::Array2D extended with a new fill method which expects a callable
  object.
---
 test/api/test_separable_filter_2d.cpp | 24 +++++++++----------
 test/framework/array.h                | 21 +++++++++++++++++
 test/framework/kernel.h               |  5 +++-
 test/framework/utils.h                | 34 +++++++++++++++++++++++++++
 4 files changed, 71 insertions(+), 13 deletions(-)

diff --git a/test/api/test_separable_filter_2d.cpp b/test/api/test_separable_filter_2d.cpp
index 4d440b08c..4da5b6b46 100644
--- a/test/api/test_separable_filter_2d.cpp
+++ b/test/api/test_separable_filter_2d.cpp
@@ -20,7 +20,7 @@ struct SeparableFilter2DKernelTestParams;
 template <size_t KernelSize>
 struct SeparableFilter2DKernelTestParams<uint8_t, KernelSize> {
   using InputType = uint8_t;
-  using IntermediateType = uint64_t;
+  using IntermediateType = uint8_t;
   using OutputType = uint8_t;
 
   static constexpr size_t kKernelSize = KernelSize;
@@ -133,19 +133,19 @@ TYPED_TEST_SUITE(SeparableFilter2D, ElementTypes);
 // Tests kleidicv_separable_filter_2d_<input_type> API.
 TYPED_TEST(SeparableFilter2D, 5x5) {
   using KernelTestParams = SeparableFilter2DKernelTestParams<TypeParam, 5>;
-  // 5x5 SeparableFilter2D operator.
+
+  const uint8_t kernel_x[5] = {5, 0, 1, 2, 2};
+  const uint8_t kernel_y[5] = {1, 4, 3, 1, 0};
+
+  // Mask is created by 'kernel_y (outer product) kernel_x'
   test::Array2D<typename KernelTestParams::IntermediateType> mask{5, 5};
-  // clang-format off
-  mask.set(0, 0, { 4, 2, 0, 4, 2});
-  mask.set(1, 0, { 2, 1, 0, 2, 1});
-  mask.set(2, 0, { 0, 0, 0, 0, 0});
-  mask.set(3, 0, { 4, 2, 0, 4, 2});
-  mask.set(4, 0, { 2, 1, 0, 2, 1});
-  // clang-format on
-  uint8_t kernel[5] = {2, 1, 0, 2, 1};
-  SeparableFilter2DTest<KernelTestParams>{kernel, kernel}
+  mask.fill([&](size_t row, size_t column) {
+    return kernel_y[row] * kernel_x[column];
+  });
+
+  SeparableFilter2DTest<KernelTestParams>{kernel_x, kernel_y}
       .with_border_types(make_generator_ptr(kAllBorders))
-      .test(mask, 7);
+      .test(mask, 5);
 }
 
 TYPED_TEST(SeparableFilter2D, 5x5Overflow) {
diff --git a/test/framework/array.h b/test/framework/array.h
index 7ad94af55..8de46cf34 100644
--- a/test/framework/array.h
+++ b/test/framework/array.h
@@ -10,6 +10,7 @@
 #include <cstddef>
 #include <cstdint>
 #include <cstring>
+#include <functional>
 #include <limits>
 #include <memory>
 #include <optional>
@@ -124,6 +125,26 @@ class Array2D : public TwoDimensional<ElementType> {
     }
   }
 
+  // Fills the underlying memory range with the output of a caller provided
+  // callable object skipping padding bytes.
+  void fill(
+      std::function<std::optional<ElementType>(size_t, size_t)> value_at) {
+    ASSERT_EQ(valid(), true);
+
+    ElementType *ptr = data();
+    for (size_t row = 0; row < height(); ++row) {
+      for (size_t column = 0; column < width(); ++column) {
+        std::optional<ElementType> optional_value = value_at(row, column);
+        ASSERT_NE(optional_value, std::nullopt);
+        if (optional_value.has_value()) {
+          ptr[column] = optional_value.value();
+        }
+      }
+
+      ptr = add_stride(ptr, 1);
+    }
+  }
+
   // Sets values in a row starting at a given column.
   void set(size_t row, size_t column,
            std::initializer_list<ElementType> values) {
diff --git a/test/framework/kernel.h b/test/framework/kernel.h
index 9fb8f9c98..94ce83439 100644
--- a/test/framework/kernel.h
+++ b/test/framework/kernel.h
@@ -15,6 +15,7 @@
 #include "framework/border.h"
 #include "framework/generator.h"
 #include "framework/types.h"
+#include "framework/utils.h"
 #include "kleidicv/kleidicv.h"
 
 namespace test {
@@ -186,7 +187,9 @@ class KernelTest {
         IntermediateType coefficient = kernel.at(height, width)[0];
         InputType value =
             source.at(row + height, column + width * source.channels())[0];
-        result += coefficient * static_cast<IntermediateType>(value);
+        result = saturating_add(
+            result,
+            saturating_mul(coefficient, static_cast<IntermediateType>(value)));
       }
     }
 
diff --git a/test/framework/utils.h b/test/framework/utils.h
index c68024d8e..622dc527a 100644
--- a/test/framework/utils.h
+++ b/test/framework/utils.h
@@ -10,6 +10,7 @@
 #include <cstdint>
 #include <cstdlib>
 #include <iostream>
+#include <limits>
 #include <tuple>
 #include <type_traits>
 
@@ -172,6 +173,39 @@ void test_null_args(Function f, Args... args) {
   Tester::template test<LastArgIndex>(f, Tuple(args...));
 }
 
+template <typename T>
+T saturating_add(T a, T b) {
+  T result;
+  if (__builtin_add_overflow(a, b, &result)) {
+    if constexpr (std::is_unsigned_v<T>) {
+      result = std::numeric_limits<T>::max();
+    } else {
+      result =
+          b < 0 ? std::numeric_limits<T>::min() : std::numeric_limits<T>::max();
+    }
+  }
+
+  return result;
+}
+
+template <typename T>
+T saturating_mul(T a, T b) {
+  T result;
+  if (__builtin_mul_overflow(a, b, &result)) {
+    if constexpr (std::is_unsigned_v<T>) {
+      result = std::numeric_limits<T>::max();
+    } else {
+      if ((a < 0 && b < 0) || (a > 0 && b > 0)) {
+        result = std::numeric_limits<T>::max();
+      } else {
+        result = std::numeric_limits<T>::min();
+      }
+    }
+  }
+
+  return result;
+}
+
 }  // namespace test
 
 #endif  // KLEIDICV_TEST_FRAMEWORK_UTILS_H_
-- 
GitLab