Support reading and writing bitpacked activations in C++ kernels.

[AdamHillier]

What do these changes do?

This feature is disabled by default and so does not change any behaviour for new or existing converted models; enabling this feature will require changes to the converter to set the correct op flags.

Currently, binary convolutions expect full precision input, bitpack that input, perform the computation, and then write full-precision output. This feature allows a binary convolutions to instead perform bitpacking inside the kernel and write (8-bit or 32-bit) bitpacked output, and for the subsequent binary convolution to skip the usual initial bitpacking step and read the already-bitpacked input directly. Doing this significantly reduces the number of reads and writes to memory and the the overall memory footprint of the model inference.

Support is added only for the C++ kernels (x86 and Arm32), including the reference kernel. Support for the optimised assembly kernels (Arm64) requires additional future work.

How have these changes been tested?

The existing bconv2d_test.cc tests have been extended to support testing bitpacked input/output tensors with the Ruy c++ kernel (8-bit bitpacking) and the reference kernel (32-bit bitpacking).

[AdamHillier]

Thank you very much for reviewing @Tombana, and your suggestions. I've now merged in changes from master and the PR is now extended to work with the reference kernel as well (which took far, far, far longer than I expected it would).

A few remarks:

• In the reference kernel 32-bit bitpacking is used; in the C++ Ruy kernel 8-bit bitpacking is used. This now means that reference and optimised ops cannot be used simultaneously in the same interpreter, because they won't interact correctly when using bitpacked activations. This shouldn't ever actually be a problem because mixing the two wouldn't make sense, but it's not a restriction that we actually enforce anywhere and technically did work previously.

• One problem I encountered with 32-bit bitpacking in the reference kernel was being forced to use the kTFLiteInt32 type for bitpacked tensors, as TFL doesn't have an unsigned 32 bit integer type. Unfortunately, this conflicted with using std::uint8_t and std::uint32_t as destination scalars throughout my implementation to template the bitpacked versions of the kernels. TFLite didn't like me populating a kTFLiteInt32 tensor with std::uint32_t data, or extracting std::uint32_t data from the tensor. Rather than searching for a cumbersome workaround, I elected to change all std::uint8_t and std::uint32_t usage as destination scalars to the signed equivalents std::uint8 and std::uint32.

  This means there is now a bit of a mis-match between TBitpacked and DstScalar, because in the former we still used the unsigned types (although I don't think there's a technical restriction given that the bitpacking functions use std::make_unsigned, and so would also work with the signed scalars).

  How do you feel about switching to the signed bitpacking types everywhere?

• It will probably be easiest to review the new changes commit-by-commit.

[Tombana]

• In the reference kernel 32-bit bitpacking is used; in the C++ Ruy kernel 8-bit bitpacking is used. This now means that reference and optimised ops cannot be used simultaneously in the same interpreter, because they won't interact correctly when using bitpacked activations. This shouldn't ever actually be a problem because mixing the two wouldn't make sense, but it's not a restriction that we actually enforce anywhere and technically did work previously.

It should indeed not be a problem. In the long-term however, I think we have to try to get the 32- (or even 64-)bit RUY version working (it already does, but simply not multithreaded), because it will be faster because of memory-alignment. I think we can do that in a separate PR, later.

• One problem I encountered with 32-bit bitpacking in the reference kernel was being forced to use the kTFLiteInt32 type for bitpacked tensors, as TFL doesn't have an unsigned 32 bit integer type. Unfortunately, this conflicted with using std::uint8_t and std::uint32_t as destination scalars throughout my implementation to template the bitpacked versions of the kernels. TFLite didn't like me populating a kTFLiteInt32 tensor with std::uint32_t data, or extracting std::uint32_t data from the tensor. Rather than searching for a cumbersome workaround, I elected to change all std::uint8_t and std::uint32_t usage as destination scalars to the signed equivalents std::uint8 and std::uint32.
  This means there is now a bit of a mis-match between TBitpacked and DstScalar, because in the former we still used the unsigned types (although I don't think there's a technical restriction given that the bitpacking functions use std::make_unsigned, and so would also work with the signed scalars).

  How do you feel about switching to the signed bitpacking types everywhere?

I've ran into the same problems, its very annoying that you can't put uint32 data into a tflite tensor without reinterpret-casting it to int32.
Given that one could forget about std::make_unsigned (which can cause hard-to-debug problems), I vote for an entirely new datatype, now that we are getting int8 quantized types as well. The RFC is here: #325
This does not solve the issue of the tflite tensor type enum, but at least our code will be much clearer and readable.

• It will probably be easiest to review the new changes commit-by-commit.

Thanks!

[AdamHillier]

GitHub won't let me approve my own PR, even if a substantial part of it was written by someone else, but thank you very much @Tombana for all of this 🎉