-
Notifications
You must be signed in to change notification settings - Fork 33
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
Labels
internal-improvement
Internal Improvements and Maintenance
Comments
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
As described in #305 we run into problems of overlapping datatypes.
At first we had
float
for normal data, anduint8
,uint32
anduint64
for bitpacked data. Now we addint8
for 8-bit quantized data, so this would still be fine without overlap of datatypes, but later we might have to adduint8
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 extrastd::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:
Usage:
Or
With the
explicit
constructor, we avoid accidentally casting a normal int to a packed datatype: if we have anint x
then a function will acceptfunc( Packed(x) )
but notfunc( x )
.The
operator T()
should be discussed. Having it has the advantage that you can usePacked<32> x
as ifx
were an int, without any runtime cost. For example you can dox ^ y
to get the xor. However, this also means that if a function expects a regular int, you can pass in aPacked<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 withuint32_t
.The text was updated successfully, but these errors were encountered: