Skip to content
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.

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

CpuGemmConv2d operator has incorrect setting for INT8_NHWC Data. #1039

Closed
GGGGxxxxxxxxr opened this issue Mar 20, 2023 · 1 comment
Closed
Assignees
Labels

Comments

@GGGGxxxxxxxxr
Copy link

GGGGxxxxxxxxr commented Mar 20, 2023

Hi!

I have just configured the NEConvolutionLayer with NHWC_QASYMM8_SIGNED Tensor, and I have found there are two issues here:

  1. All the INT8 NHWC Convolution Operator would not skip reshaping kernel after GEMM, which does not make any sense, as GEMMOUTPUT equals to the output tensor of layer in terms of continuous data within memory under the layout of NHWC.
  2. 1x1 ConvolutionKernel would still call Im2Col kernel, which does not make sense. 1x1 Kernel under NHWC layout should be simple GEMM after weight reshaping.

The previous two problems are caused by skip_im_col_info() function in CpuGemmConv2d.cpp, and the actual problem is in the lower part of CpuGemmLowpMatrixMultiplyCore::Validate().

3*.The reshaping kernel itself has been poorly implemented under NHWC layout in kernel level, as the value assignment is implemented value by value, which could be easily optimized with MEMCPY() row by row, just simply setting WindowDim::x as (0,1,1) and set the corresponding iterator.

I have modified this problem myself in CpuGemmConv2d.cpp and I have tested with random numbers, the result is mathematically correct which I have verified, I suppose this issue should be fixed, otherwise the performance for this setting would be greatly affected.

@morgolock morgolock self-assigned this Mar 24, 2023
@morgolock morgolock added this to the v23.05 milestone Mar 24, 2023
@morgolock morgolock removed this from the v23.05 milestone Jun 3, 2024
@morgolock
Copy link

Closing this due to the lack of activity. If this is still a problem please reopen.

It would be helpful if you could provide a standalone test reproducing the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants