Skip to content

Commit 7844978

Browse files
committed
fix: Don't skip Im2Col if convolution has paddings
Resolves: ARMCL-1254 Issue: #1253 Change-Id: I8c88dde1fd4df69f4a7eaf8ad4125462c09dda3f Signed-off-by: Gunes Bayir <gunes.bayir@arm.com>
1 parent cf751ff commit 7844978

File tree

3 files changed

+18
-11
lines changed

3 files changed

+18
-11
lines changed

src/cpu/operators/CpuGemmConv2d.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2021-2025 Arm Limited.
2+
* Copyright (c) 2021-2026 Arm Limited.
33
*
44
* SPDX-License-Identifier: MIT
55
*
@@ -185,8 +185,10 @@ CpuGemmConv2d::SkipInfo CpuGemmConv2d::skip_im_col_info(const ITensorInfo
185185
unsigned int conv_h = 0;
186186
std::tie(conv_w, conv_h) = scaled_dimensions(src->dimension(idx_width), src->dimension(idx_height), kernel_width,
187187
kernel_height, conv_info, dilation);
188-
const bool skip_im2col = (data_layout == DataLayout::NHWC && kernel_width == 1 && kernel_height == 1 &&
189-
conv_info.stride().first == 1 && conv_info.stride().second == 1);
188+
189+
const bool skip_im2col = (data_layout == DataLayout::NHWC && kernel_width == 1 && kernel_height == 1 &&
190+
conv_info.stride().first == 1 && conv_info.stride().second == 1) &&
191+
!conv_info.has_padding();
190192

191193
if (skip_im2col)
192194
{

src/gpu/cl/operators/ClGemmConv2d.cpp

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017-2021, 2023 Arm Limited.
2+
* Copyright (c) 2017-2021, 2023, 2026 Arm Limited.
33
*
44
* SPDX-License-Identifier: MIT
55
*
@@ -214,8 +214,9 @@ void ClGemmConv2d::configure(const CLCompileContext &compile_context,
214214
_is_prepared = weights_info.retain_internal_weights();
215215
_is_quantized = is_data_type_quantized_asymmetric(src->data_type());
216216
_skip_im2col = (data_layout == DataLayout::NHWC && kernel_width == 1 && kernel_height == 1 &&
217-
conv2d_info.conv_info.stride().first == 1 && conv2d_info.conv_info.stride().second == 1);
218-
_skip_col2im = data_layout == DataLayout::NHWC;
217+
conv2d_info.conv_info.stride().first == 1 && conv2d_info.conv_info.stride().second == 1) &&
218+
!conv2d_info.conv_info.has_padding();
219+
_skip_col2im = data_layout == DataLayout::NHWC;
219220

220221
// Only for quantize there are few cases where we cannot fuse the activation function in GEMM
221222
_fuse_activation = true;
@@ -419,10 +420,12 @@ Status ClGemmConv2d::validate(const ITensorInfo *src,
419420
const ITensorInfo *gemm_output_to_use = dst;
420421
const ITensorInfo *weights_to_use = weights;
421422
const bool is_quantized = is_data_type_quantized_asymmetric(data_type);
422-
const bool skip_im2col = (data_layout == DataLayout::NHWC && kernel_width == 1 && kernel_height == 1 &&
423-
conv2d_info.conv_info.stride().first == 1 && conv2d_info.conv_info.stride().second == 1);
424-
const bool skip_col2im = data_layout == DataLayout::NHWC;
425-
bool fuse_activation = true;
423+
const bool skip_im2col =
424+
(data_layout == DataLayout::NHWC && kernel_width == 1 && kernel_height == 1 &&
425+
conv2d_info.conv_info.stride().first == 1 && conv2d_info.conv_info.stride().second == 1) &&
426+
!conv2d_info.conv_info.has_padding();
427+
const bool skip_col2im = data_layout == DataLayout::NHWC;
428+
bool fuse_activation = true;
426429

427430
ARM_COMPUTE_RETURN_ERROR_ON((weights->dimension(idx_channel) * conv2d_info.num_groups) !=
428431
src->dimension(idx_channel));

tests/datasets/SmallConvolutionLayerDataset.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017-2021, 2025 Arm Limited.
2+
* Copyright (c) 2017-2021, 2025-2026 Arm Limited.
33
*
44
* SPDX-License-Identifier: MIT
55
*
@@ -181,6 +181,8 @@ class SmallConvolutionLayerDataset final : public ConvolutionLayerDataset
181181
// 1x1 Kernel with Stride (1, 1) and NHWC data layout in order to test skipping Im2Col
182182
add_config(TensorShape(1U, 5U, 2U), TensorShape(1U, 1U, 2U, 3U), TensorShape(3U), TensorShape(1U, 5U, 3U),
183183
PadStrideInfo(1, 1, 0, 0));
184+
add_config(TensorShape(1U, 5U, 2U), TensorShape(1U, 1U, 2U, 3U), TensorShape(3U), TensorShape(3U, 7U, 3U),
185+
PadStrideInfo(1, 1, 1, 1));
184186

185187
// Batch size 1
186188
add_config(TensorShape(23U, 27U, 5U), TensorShape(3U, 3U, 5U, 2U), TensorShape(2U), TensorShape(11U, 25U, 2U),

0 commit comments

Comments
 (0)