Skip to content

Conversation

@Anndrey24
Copy link
Contributor

@Anndrey24 Anndrey24 commented Aug 31, 2023

The Legalize pass was unnecessarily padding the input channels for conv2d int8 native implementations. Since the conv2d schedule itself can add padding more efficiently, I skipped padding during the pass and further optimised the schedule to deal with it.
I also added a test to check whether or not the Legalize pass pads the conv2d input data.

The benchmark results of a single int8 conv2d operation where input padding along the K axis is necessary, with input = (1 x 224 x 224 x 3), filter = (16 x 3 x 3 x 3), output = (1 x 112 x 112 x 16), tested for the following target "llvm --device=arm_cpu --mtriple=aarch64-linux-gnu -mattr=+v8.2a,+dotprod" , before and after the changes, is given below:

Before (ms) After (ms) Speedup (%)
0.5186 0.0663 87.22

…edule in Legalize pass

The Legalize pass was unnecessarily padding the input channels for conv2d int8 native implementations. Since the conv2d schedule itself can add padding more efficiently, I skipped padding during the pass and further optimised the schedule to deal with it.
For the int8 interleaved implementation, I kept the Legalize pass padding, which transforms the input channels into a multiple of 8, and modified the schedule to ensure vectorization of the input data.
I also added a test to check whether or not the Legalize pass pads the conv2d input data.
Copy link
Contributor

@lhutton1 lhutton1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM! I just had a couple of comments to improve readability of the schedules. It would also be good to check the schedule tests cover the cases that have been added

@Anndrey24
Copy link
Contributor Author

Anndrey24 commented Sep 5, 2023

Regarding the tests, it seems that all of the padding cases are already checked at least once in:

def test_conv2d_NHWC_gemm_int8(params, device):

@lhutton1
Copy link
Contributor

lhutton1 commented Sep 5, 2023

Awesome, thanks for checking!

Copy link
Contributor

@lhutton1 lhutton1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Anndrey24! Will leave open for a bit incase @ekalda wants to take a look

@lhutton1 lhutton1 merged commit a25843b into apache:main Sep 13, 2023
@lhutton1
Copy link
Contributor

Thanks @Anndrey24!

@Anndrey24 Anndrey24 deleted the conv2d-legalize branch November 8, 2023 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants