COMPMID-3708 Remove OpenCL padding: CLCopyKernel [Patch1]

* Remove padding only for when user-supplied padding is empty
    * Vectorize the case where output_window is not null and the output
    window is narrow in x (smaller than vec_size_x)

Change-Id: I313089fe309e87e8529ecfd00542fcfa4dc44862
Signed-off-by: SiCong Li <sicong.li@arm.com>
Reviewed-on: https://review.mlplatform.org/c/ml/ComputeLibrary/+/4193
Reviewed-by: Gian Marco Iodice <gianmarco.iodice@arm.com>
Tested-by: Arm Jenkins <bsgcomp@arm.com>
Comments-Addressed: Arm Jenkins <bsgcomp@arm.com>

3 files changed, 39 insertions, 55 deletions

diff --git a/src/core/CL/cl_kernels/copy_tensor.cl b/src/core/CL/cl_kernels/copy_tensor.cl
index 0592e07511..95da9a3cd3 100644
--- a/src/core/CL/cl_kernels/copy_tensor.cl
+++ b/src/core/CL/cl_kernels/copy_tensor.cl
@@ -77,9 +77,14 @@ __kernel void copy_pad_tensor(
 }
 #endif // Compile time constants
 
-#if defined(DATA_TYPE)
+#if defined(DATA_TYPE) && defined(VEC_SIZE) && defined(VEC_SIZE_LEFTOVER)
 /** Performs a copy of input tensor to the output tensor.
  *
+ * @note The following variables must be passed at compile time:
+ * -# -DDATA_TYPE        : Input and output datatypes.
+ * -# -DVEC_SIZE         : The number of elements processed in X dimension
+ * -# -DVEC_SIZE_LEFTOVER: Leftover size in the X dimension; x_dimension % VEC_SIZE
+ *
  * @param[in]  in_ptr                            Pointer to the source tensor. Supported data types: All
  * @param[in]  in_stride_x                       Stride of the source tensor in X dimension (in bytes)
  * @param[in]  in_step_x                         input_stride_x * number of elements along X processed per workitem(in bytes)
@@ -104,25 +109,18 @@ __kernel void copy_tensor(
     Tensor3D in  = CONVERT_TO_TENSOR3D_STRUCT(in);
     Tensor3D out = CONVERT_TO_TENSOR3D_STRUCT(out);
 
-#if defined(VEC_SIZE)
-
-#if defined(LAST_ACCESSED_X)
-    // Check if access on width gets out of bounds
-    // If it does then shift access vector to access elements within bounds
-    const int shift = max((int)(get_global_id(0) * VEC_SIZE) - (int)LAST_ACCESSED_X, 0);
+    // Boundary-aware access:
+    // If the there's left-over in width (VEC_SIZE_LEFTOVER > 0):
+    // Shift all accesses other than the first to avoid accessing out of bounds
+    const int shift = max((int)(get_global_id(0) * VEC_SIZE) - (int)VEC_SIZE_LEFTOVER, 0) % VEC_SIZE;
     in.ptr -= shift * in.stride_x;
     out.ptr -= shift * out.stride_x;
-#endif // defined(LAST_ACCESSED_X)
 
     // Load data
     VEC_DATA_TYPE(DATA_TYPE, VEC_SIZE)
-    data = VLOAD(VEC_SIZE)(0, (__global DATA_TYPE *)in.ptr);
+    data0 = VLOAD(VEC_SIZE)(0, (__global DATA_TYPE *)in.ptr);
 
-    // Store result
-    VSTORE(VEC_SIZE)
-    (data, 0, (__global DATA_TYPE *)out.ptr);
-#else  // defined(VEC_SIZE)
-    *((__global DATA_TYPE *)(out.ptr)) = *((__global DATA_TYPE *)(in.ptr));
-#endif // defined(VEC_SIZE)
+    // Boundary-aware store
+    STORE_VECTOR_SELECT(data, DATA_TYPE, (__global DATA_TYPE *)out.ptr, VEC_SIZE, VEC_SIZE_LEFTOVER, VEC_SIZE_LEFTOVER != 0 && get_global_id(0) == 0);
 }
-#endif // defined(DATA_TYPE)
\ No newline at end of file
+#endif // defined(DATA_TYPE) && defined(VEC_SIZE) && defined(VEC_SIZE_LEFTOVER)
\ No newline at end of file
diff --git a/src/core/CL/kernels/CLCopyKernel.cpp b/src/core/CL/kernels/CLCopyKernel.cpp
index 0b7e9aff53..769f15de0f 100644
--- a/src/core/CL/kernels/CLCopyKernel.cpp
+++ b/src/core/CL/kernels/CLCopyKernel.cpp
@@ -28,6 +28,7 @@
 #include "arm_compute/core/CL/ICLTensor.h"
 #include "arm_compute/core/Helpers.h"
 #include "arm_compute/core/TensorInfo.h"
+#include "arm_compute/core/Utils.h"
 #include "arm_compute/core/utils/misc/ShapeCalculator.h"
 #include "src/core/AccessWindowStatic.h"
 #include "src/core/helpers/AutoConfiguration.h"
@@ -63,32 +64,16 @@ Status validate_arguments(const ITensorInfo *input, const ITensorInfo *output, c
     return Status{};
 }
 
-std::pair<Status, Window> validate_and_configure_window(ITensorInfo *input, ITensorInfo *output, Window *output_window)
+std::pair<Status, Window> configure_window(ITensorInfo *input, ITensorInfo *output)
 {
     // Output auto inizialitation if not yet initialized
     auto_init_if_empty(*output, *input);
 
     // Configure window
-    const unsigned int vec_size_x = 16 / input->element_size();
+    const unsigned int vec_size_x = adjust_vec_size(16 / input->element_size(), input->dimension(0));
 
-    if(output_window == nullptr)
-    {
-        // Create and update the window (if needed)
-        Window win = calculate_max_window(*input, Steps(vec_size_x));
-
-        AccessWindowHorizontal input_access(input, 0, vec_size_x);
-        AccessWindowHorizontal output_access(output, 0, vec_size_x);
-
-        bool window_changed = update_window_and_padding(win, input_access, output_access);
-
-        Status err = (window_changed) ? ARM_COMPUTE_CREATE_ERROR(ErrorCode::RUNTIME_ERROR, "Insufficient Padding!") : Status{};
-        return std::make_pair(err, win);
-    }
-    else
-    {
-        Window win = calculate_max_window(*input);
-        return std::make_pair(Status{}, win);
-    }
+    const Window win = calculate_max_window(*input, Steps(vec_size_x));
+    return std::make_pair(Status{}, win);
 }
 
 std::pair<Status, Window> validate_and_configure_window_with_padding(ITensorInfo *input, ITensorInfo *output, const PaddingList &padding)
@@ -165,6 +150,8 @@ void CLCopyKernel::configure(const CLCompileContext &compile_context, const ICLT
     ARM_COMPUTE_ERROR_ON_NULLPTR(input, output);
     ARM_COMPUTE_ERROR_THROW_ON(validate_arguments(input->info(), output->info(), padding, output_window));
 
+    auto padding_info = get_padding_info({ input, output });
+
     _input  = input;
     _output = output;
 
@@ -179,30 +166,33 @@ void CLCopyKernel::configure(const CLCompileContext &compile_context, const ICLT
     if(padding.empty())
     {
         // Configure window
-        win_config = validate_and_configure_window(input->info(), output->info(), output_window);
+        win_config = configure_window(input->info(), output->info());
 
         if(output_window != nullptr)
         {
-            _has_output_window        = true;
-            _output_window            = Window(*output_window);
-            const int  width_x        = output_window->num_iterations(0);
-            const bool multi_access_x = width_x >= static_cast<int32_t>(vec_size_x);
-            const bool remainder_x    = width_x % vec_size_x > 0;
+            _has_output_window             = true;
+            _output_window                 = Window(*output_window);
+            const int  width_x             = output_window->num_iterations(0);
+            const int  vec_size_x_leftover = width_x % vec_size_x;
+            const bool multi_access_x      = width_x >= static_cast<int32_t>(vec_size_x);
 
             if(multi_access_x)
             {
                 _output_window.set(Window::DimX, Window::Dimension(output_window->x().start(), ceil_to_multiple(output_window->x().end(), vec_size_x), vec_size_x));
-                win_config.second.set(Window::DimX, Window::Dimension(win_config.second.x().start(), ceil_to_multiple(win_config.second.x().end(), vec_size_x), vec_size_x));
             }
 
-            build_opts.add_option_if(multi_access_x, "-DVEC_SIZE=" + support::cpp11::to_string(vec_size_x));
-            build_opts.add_option_if(multi_access_x && remainder_x, "-DLAST_ACCESSED_X=" + support::cpp11::to_string(std::max<int>(width_x - vec_size_x, 0)));
+            build_opts.add_option("-DVEC_SIZE_LEFTOVER=" + support::cpp11::to_string(vec_size_x_leftover));
         }
         else
         {
-            build_opts.add_option("-DVEC_SIZE=" + support::cpp11::to_string(vec_size_x));
+            const int width_x             = input->info()->tensor_shape().x();
+            const int vec_size_x_leftover = width_x % vec_size_x;
+
+            build_opts.add_option("-DVEC_SIZE_LEFTOVER=" + support::cpp11::to_string(vec_size_x_leftover));
         }
 
+        build_opts.add_option("-DVEC_SIZE=" + support::cpp11::to_string(vec_size_x));
+
         // Build kernel
         _kernel = create_kernel(compile_context, "copy_tensor", build_opts.options());
     }
@@ -231,17 +221,15 @@ void CLCopyKernel::configure(const CLCompileContext &compile_context, const ICLT
     // Validate and set the window
     ARM_COMPUTE_ERROR_THROW_ON(win_config.first);
     ICLKernel::configure_internal(win_config.second);
+
+    ARM_COMPUTE_ERROR_ON(has_padding_changed(padding_info));
 }
 
 Status CLCopyKernel::validate(const arm_compute::ITensorInfo *input, const arm_compute::ITensorInfo *output, const PaddingList &padding, Window *output_window)
 {
     ARM_COMPUTE_RETURN_ON_ERROR(validate_arguments(input, output, padding, output_window));
 
-    if(padding.empty())
-    {
-        ARM_COMPUTE_RETURN_ON_ERROR(validate_and_configure_window(input->clone().get(), output->clone().get(), output_window).first);
-    }
-    else
+    if(!padding.empty())
     {
         ARM_COMPUTE_RETURN_ON_ERROR(validate_and_configure_window_with_padding(input->clone().get(), output->clone().get(), padding).first);
     }
diff --git a/tests/validation/CL/Copy.cpp b/tests/validation/CL/Copy.cpp
index 07af24352e..0b2a15146b 100644
--- a/tests/validation/CL/Copy.cpp
+++ b/tests/validation/CL/Copy.cpp
@@ -48,15 +48,13 @@ TEST_SUITE(Copy)
 DATA_TEST_CASE(Validate, framework::DatasetMode::ALL, zip(zip(
                framework::dataset::make("InputInfo", { TensorInfo(TensorShape(32U, 13U, 2U), 1, DataType::U8),  // Invalid data type combination
                                                        TensorInfo(TensorShape(32U, 13U, 2U), 1, DataType::U8),  // Mismatching shapes
-                                                       TensorInfo(TensorShape(14U, 13U, 2U), 1, DataType::U8),  // Window shrink
-                                                       TensorInfo(TensorShape(32U, 32U, 2U), 1, DataType::U8),
+                                                       TensorInfo(TensorShape(14U, 13U, 2U), 1, DataType::U8),
                                                      }),
                framework::dataset::make("OutputInfo",{ TensorInfo(TensorShape(32U, 13U, 2U), 1, DataType::S16),
                                                        TensorInfo(TensorShape(32U, 11U, 2U), 1, DataType::U8),
                                                        TensorInfo(TensorShape(14U, 13U, 2U), 1, DataType::U8),
-                                                       TensorInfo(TensorShape(32U, 32U, 2U), 1, DataType::U8),
                                                      })),
-               framework::dataset::make("Expected", { false, false, false, true })),
+               framework::dataset::make("Expected", { false, false, true })),
                input_info, output_info, expected)
 {
     ARM_COMPUTE_EXPECT(bool(CLCopy::validate(&input_info.clone()->set_is_resizable(false), &output_info.clone()->set_is_resizable(false))) == expected, framework::LogLevel::ERRORS);