-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[Bugfix][Relay][Strategy] Enable compile time transformation of weights matrix for arm_cpu NHWC quantized conv2d #15584
Conversation
…ts matrix for arm_cpu NHWC quantized conv2d Fixed arm_cpu strategy bug which was causing tensorization errors when using the `AlterOpLayout` pass for the quantized NHWC conv2d schedules, as discovered in apache#10724. Therefore, we can now also enable the usage of `AlterOpLayout` for these schedules in order to transform the weight matrix at compile time, instead of runtime as before. I also modified the padding in `Conv2DGemmWeightTransformRel` and `interleave_transpose_weights` to reflect the changes made in apache#13669 and updated the AlterOpLayout tests accordingly.
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment. Generated by tvm-bot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Anndrey24, this looks like a great improvement! It seems we have test coverage for the alter op layout pass which is great, although, it would be good to also have a test to check no weight transform happens at runtime. Is it possible to extend
@pytest.mark.parametrize( |
Updated the int8 conv2d implementation selection test to include more targets and to run the AlterOpLayout pass such that its respective changes of schedule are captured.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @Anndrey24! LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Anndrey24, very well spotted!
Thanks @Anndrey24 @ekalda! |
Refactored out a piece of common functionality from the `conv2d_gemm_weight_transform` and `interleave_transpose_weights` functions, which has previously led to bugs stemming from changes made to only one but not the other, like in apache#15584. Determining the necessary padding for the interleaved and transposed weights matrix has now been separated into a new utility function, allowing future changes to be reflected in both callers.
Refactored out a piece of common functionality from the `conv2d_gemm_weight_transform` and `interleave_transpose_weights` functions, which has previously led to bugs stemming from changes made to only one but not the other, like in #15584. Determining the necessary padding for the interleaved and transposed weights matrix has now been separated into a new utility function, allowing future changes to be reflected in both callers.
Fixed arm_cpu strategy bug which was causing tensorization errors when using the
AlterOpLayout
pass for the quantized NHWC conv2d schedules, as discovered in #10724. Therefore, we can now also enable the usage ofAlterOpLayout
for these schedules in order to transform the weight matrix at compile time, instead of runtime as before.I also modified the padding in
Conv2DGemmWeightTransformRel
andinterleave_transpose_weights
to reflect the changes made in #13669 and updated the AlterOpLayout tests accordingly.cc @ekalda @lhutton1 @leandron