[QNNPACK] Block Sparse kernel. First commit.

[kimishpatel]

Stack from ghstack:

• [QNNPACK Sparsity] Add aarch64 kernel of 8x1 sparsity #51120 [QNNPACK Sparsity] Add aarch64 kernel of 8x1 sparsity
• [QNNPACK, Sparsity] Add 8x1 block sparse kernels for aarch32. #51119 [QNNPACK, Sparsity] Add 8x1 block sparse kernels for aarch32.
• [QNNPACK, Sparsity] Code refactoring to allow for more generic block #51118 [QNNPACK, Sparsity] Code refactoring to allow for more generic block
• [QNNPACK, Sparsity] Add dyanmic linear sparse kernel for arm64 #50591 [QNNPACK, Sparsity] Add dyanmic linear sparse kernel for arm64
• [QNNPACK, Sparsity] Sparse kernel with 4x8 blocking #50590 [QNNPACK, Sparsity] Sparse kernel with 4x8 blocking
• [QNNPACK, Sparsity] Added prepacking base aarch32 kernels #50589 [QNNPACK, Sparsity] Added prepacking base aarch32 kernels
• [QNNPACK, Sparsity] ARMV7, aarch32, kernels for dynamic linear #50588 [QNNPACK, Sparsity] ARMV7, aarch32, kernels for dynamic linear
• [QNNPACK:Sparsity] Add A matrix pretransformed based sparse kernels for FC #50587 [QNNPACK:Sparsity] Add A matrix pretransformed based sparse kernels for FC
• [QNNPACK Sparse] Create fc sparse operator #50586 [QNNPACK Sparse] Create fc sparse operator
• [QNNPACK] Block Sparse kernel. First commit. #50585 [QNNPACK] Block Sparse kernel. First commit.

Summary:
This diff introduces sparse kenel for sse. Uses 1x4 block sparse
pattern.

Test Plan:
./build/local/q8gemm-sparse-test

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D25925500

[AshkanAliabadi]

Just pointing out that the rest of the code uses C++11, but this is OK with me personally.

[AshkanAliabadi]

I take this back, I was looking at the original file. :)

[AshkanAliabadi]

Our fork of QNNPACK is still a C only library except for the unittests. Are you OK with introducing C++ into the code?

[kimishpatel]

I should structure this better. This is not really used in any kernels, so operators wont be using this. We will use packing on cpp side. But thanks for pointing this out.

[AshkanAliabadi]

An assert would be helpful to check for preconditions. I think generous use of asserts is good practice - has no runtime cost but can come in handy when dealing with code that behaves in unexpected ways.

[AshkanAliabadi]

bcsr_mat.row_values.resize() followed by clear should not be necessary when followed by reserve() and operating on a newly constructed vector. If you just want to pre-allocate memory without modifying the vector size then a call to reserve should be enough.

[kimishpatel]

Yeah this is an oversight. Thanks for catching. Will fix it.

[AshkanAliabadi]

This doesn't matter since this is a unit test but placing anonymous or static functions in a header file means each compilation unit is going to end up with its own copy since this implies internal linkage. I am not actually sure if the compiler consolidates all [similar] functions into one in practice but avoid this usage personally.

[kimishpatel]

I think you are right. It will just create a separate copy for each translation unit. But I am more lax with test code and did this just out of practice.

[AshkanAliabadi]

This doesn't seem to be used anywhere.

[kimishpatel]

Thanks for the catch.

[AshkanAliabadi]

I personally lean on using forced inline functions if possible. The thing with macros like this is that if you end up accidentally using any arithmetic in the arguments at call-site LOAD_TRANSPOSED_A_BLOCK(a, b, c+1) this will have unexpected consequences. Also the commonality like col_block_id * COL_BLOCK_SIZE + index can be factored out more readily when using functions.

[kimishpatel]

I opted this way mainly because I can generate variables names like va0, va1 etc. that are later used and can avoid passing 10 args to the function. I agree about arithmetics used in arg leading to unexpected consequences. I will fix it appropriately.

[facebook-github-bot]

This pull request has been merged in 520f96b.