intel
Enhancement: add extra encoding/decoding half precision floating point functions to public API
It's uncommon to operate half precision floating point data on application level, while such format is widely used on transport level to minimize traffic. In such case one can need API for encoding/decoding half-precision floating point data AS more common single or double. Something like this:
CborError cbor_encode_float_as_half_float(CborEncoder *encoder, float value);
CborError cbor_encode_double_as_half_float(CborEncoder *encoder, double value);
CborError cbor_value_get_half_float_as_float(const CborValue *value, float *result);
CborError cbor_value_get_half_float_as_double(const CborValue *value, double *result);
// etc.
There is some related stuff in the private area, and I think, I can make it public.
Right, half-float is often used for storage only, while all the manipulation is done on single- or double-precision.
I agree on adding those functions, but let's put each pair on a separate .c file from cborparser.c and cborencoder.c. That way, those two remain floating-point-free.
Yes, it looks very good. I have one overall comment and two very minor. Overall, I think the float API has more value than double. Any system capable of double-precision either already has a library function to convert single to double or the FPU implements it. But there are systems without double support, or at least more costly than float. So let's explore changing the internal function and the API to return float and receive float.
Thanks for the tests, they look nice.
The two minor comments:
- Name the two files
cborparser_float.candcborenccoder_float.cplease - Add the two files to tst_cpp.
Look at the improvement made so far. But I'm in doubt now: I have used ldexpf function, which is not accessible in plain old C89 and some antique compilers.
Let's do bit manipulation directly. I wonder if there's a #define that helps us know that FP is IEEE 754. C++ has that.
Let's do bit manipulation directly
Do you mean to replace the
val = ldexpf(mant + 1024, exp - 25);
with
val = ((float)mant + 1024) * (float)(1 << (exp - 25)) (NB! in case exp > 25)
static inline float decode_halff(unsigned short half)
{
int exp = (half >> 10) & 0x1f;
int mant = half & 0x3ff;
float mantf, expf, val;
if (exp == 0) {
mantf = mant;
expf = 1.0f / (1 << 24);
val = mantf * expf;
} else if (exp != 31) {
mantf = mant + 1024.0f;
expf = exp >= 25 ? 1 << (exp - 25) : 1.0f / (1 << (25 - exp));
val = mantf * expf;
} else {
val = mant == 0 ? INFINITY : NAN;
}
return half & 0x8000 ? -val : val;
}
If that produces proper results, that's fine. I was thinking of converting the uint16_t to uint32_t first, then memcpy from that to a float, instead of doing FP calculations. If we're going to do math, I'd rather use ldexpf, which according to the standard is part of C99.
So, there are 4 ways with their pro-s and contra-s:
- use ldexp: standard-compliant, works on most platforms, but can be expensive;
- use ldexpf: needs the C99 compiler, can be less expensive (but still expensive);
- use naive math: works without std math lib (yes, sometimes linking without -lm option saves a lot of space), but error-prone and still expensive;
- direct bit manipulations: re-shifting mantissa, re-biasing exponent... Error-prone again, cheap for platforms without hardware floating-point math. But what's about platforms with non-IEEE-754 math? Well. we can end with dropping float and double from API at all:
#ifndef CBOR_NO_PLATFORM_IEEE754_FLOATING_POINT
CBOR_INLINE_API CborError cbor_value_get_float(CborEncoder *encoder, float *value);
CBOR_INLINE_API CborError cbor_encode_float(CborEncoder *encoder, float value);
// ... for half and double
#else
// Caveat lector: IEEE754 floating point representation is assumed
CBOR_INLINE_API CborError cbor_value_get_float(CborEncoder *encoder, uint32_t *value);
CBOR_INLINE_API CborError cbor_encode_float(CborEncoder *encoder, uint32_t value);
// ... for half and double
#endif
What is the preferable way?
CBOR is defined as carrying IEEE754 binary64, binary32 and binary16 content. So I like your idea.
I don't think we need to provide the uintXX_t version of the APIs for non-IEEE754 systems, though. Let people use cbor_encode_floating_point and cbor_value_get_floating_point (the latter does not exist), which operate on void *.
OK, but I think, this is the task for separate feature request. As for now, let's roll back to plain old doubles (ldexp, not ldexpf) both for cbor_value_get_half_float_as_float and cbor_value_get_half_float_as_double.
I don't think we need the ..._as_double API. Conversion from double to float can be done by user code before reaching TinyCBOR API.
Well, cbor_value_get_half_float_as_float needs a pointer to float, so user should do extra effort. Hope, nobody could try something like:
double x;
/* Oops... I know a guy, who can try it without any doubt */
cbor_value_get_half_float_as_float(cbor_value, (float*)&x);
But in name of API brevity ..._as_double should be eliminated.
If someone wants to shoot themselves in the foot, who are we to say no? :-)
Hello @x448 we have the code to encode and decode half floats. I just didn't want to add an API that uses that.
