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

RFC: Create a Packed datatype #325

Open
Tombana opened this issue Apr 3, 2020 · 1 comment
Open

RFC: Create a Packed datatype #325

Tombana opened this issue Apr 3, 2020 · 1 comment
Labels
internal-improvement Internal Improvements and Maintenance

Comments

@Tombana
Copy link
Collaborator

Tombana commented Apr 3, 2020

As described in #305 we run into problems of overlapping datatypes.
At first we had float for normal data, and uint8, uint32 and uint64 for bitpacked data. Now we add int8 for 8-bit quantized data, so this would still be fine without overlap of datatypes, but later we might have to add uint8 for 8-bit quantized data as well, causing problems.
To further complicate things, the tflite tensors only support types {uint8, int8, int32, int64 }, they don't have other unsigned versions.
In principle we could use the signed datatypes for bitpacking, there's not fundamental difference. One annoying this is that std::numeric_limits<T> returns 7, 31 and 63 bits, respectively, for the signed types, which is why we needed an extra std::make_unsigned in our bitpacking code.

With the int8-quantization coming up, it makes a lot of sense to simply use a new type for our bitpacked data. This will greatly improve readability of the code, and avoid mistakes. For example, if our bitpacking functions only accept a Bitpacked8 datatype, then you can not accidentally pass in an 8-bit quantized tensor. Vice versa, if a function expects an 8-bit quantized tensor, you can not accidentally pass in an 8-bit-bitpacked tensor.

The new type Packed<n> can be defined as follows:

core/packed.h:

namespace detail {

template <int B> struct underlying_type {};

template <> struct underlying_type<8> { using type = std::uint8_t; };

template <> struct underlying_type<32> { using type = std::uint32_t; };

template <> struct underlying_type<64> { using type = std::uint64_t; };

} // namespace detail


template <int B> struct Packed {
  using T = typename detail::underlying_type<B>::type;

  explicit Packed(const T x) : bits(x) {}

  static constexpr int bitwidth = B;

  operator T() { return bits; }

  T bits;
};

Usage:

template <typename T>
void foo(T x) {
  cout << "Bitwidth = " << T::bitwidth << endl;
  // use x as if its a regular uint
}

Or

template <int b> void foo(Packed<b> x) {
  cout << "Bitwidth = " << b << endl;
  cout << "Bitwidth = " << x.bitwidth << endl;
  // use x as if its a regular uint
}
  • With the explicit constructor, we avoid accidentally casting a normal int to a packed datatype: if we have an int x then a function will accept func( Packed(x) ) but not func( x ).

  • The operator T() should be discussed. Having it has the advantage that you can use Packed<32> x as if x were an int, without any runtime cost. For example you can do x ^ y to get the xor. However, this also means that if a function expects a regular int, you can pass in a Packed<32> and it will automatically cast. We might not want this, for the reasons described above: if a function expects an 8-bit quantized tensor, we don't want to allow 8-bit bitpacked tensors.
    Seeing as we never directly use it as an int except for the xor, we can also overload only the ^ xor operator and not allow any other implicit casts. (This has my preference)

  • This construction has no runtime or binary-size overhead. This can be seen in the compiler explorer, where the function with Packed<32> has exactly the same generated assembly as the function with uint32_t .

@AdamHillier
Copy link
Contributor

I'm not experienced enough with C++ types and casting behaviour to really comment on what the right implementation of this would be, but I agree that it would be very nice to have an explicit bitpacked datatype - I think it would make the code a lot clearer - so I'm in favour of the idea in principle 👍

Tombana pushed a commit that referenced this issue Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal-improvement Internal Improvements and Maintenance
Projects
None yet
Development

No branches or pull requests

2 participants