| Message ID | 20260918120949.191668-4-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Barnabás Pőcze (2026-09-18 13:09:31) > If more than 24 bits would be used, use `double` as not all integers above > 2^24 can be exactly represented by an IEEE754 binary32. > > Also add a static assertion that IEEE754 floating point types are used. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/ipa/libipa/fixedpoint.h | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/src/ipa/libipa/fixedpoint.h b/src/ipa/libipa/fixedpoint.h > index 66863075c6..0640605bee 100644 > --- a/src/ipa/libipa/fixedpoint.h > +++ b/src/ipa/libipa/fixedpoint.h > @@ -25,19 +25,20 @@ private: > static constexpr unsigned int bits = I + F; > static_assert(bits <= sizeof(UT) * 8, "FixedPointQTraits: too many bits for type UT"); > > - /* > - * If fixed point storage is required with more than 24 bits, consider > - * updating this implementation to use double-precision floating point. > - */ > - static_assert(bits <= 24, "Floating point precision may be insufficient for more than 24 bits"); > + /* IEEE754 binary64 can faithfully represent integers up to 2^53 */ > + static_assert(bits <= 53, "Floating point precision may be insufficient"); > > static constexpr UT bitMask = bits < sizeof(UT) * 8 > ? (UT{ 1 } << bits) - 1 > : ~UT{ 0 }; > > public: > + /* IEEE754 binary32 can faithfully represent integers up to 2^24 */ > using QuantizedType = UT; > - using FloatingType = float; > + using FloatingType = std::conditional_t<bits <= 24, float, double>; Ohhhh that's clever. I like it. > + > + static_assert(std::numeric_limits<FloatingType>::is_iec559, > + "requires IEEE754 floating point"); https://en.cppreference.com/cpp/types/numeric_limits/is_iec559 makes it sound reasonable. > > static constexpr UT qMin = std::is_signed_v<T> > ? -(UT{ 1 } << (bits - 1)) > @@ -88,7 +89,7 @@ namespace details { > template<unsigned int Bits> > constexpr auto qtype() > { > - static_assert(Bits <= 32, > + static_assert(Bits <= 64, > "Unsupported number of bits for quantized type"); > > if constexpr (Bits <= 8) > @@ -97,6 +98,8 @@ constexpr auto qtype() > return int16_t(); > else if constexpr (Bits <= 32) > return int32_t(); > + else if constexpr (Bits <= 64) > + return int64_t(); I wonder what we'll need to represent as such a large number, and how that might impact 32 bit systems but I think this is valid so: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > } > > } /* namespace details */ > -- > 2.55.0 >
Hi Barnabás On Fri, Sep 18, 2026 at 02:09:31PM +0200, Barnabás Pőcze wrote: > If more than 24 bits would be used, use `double` as not all integers above > 2^24 can be exactly represented by an IEEE754 binary32. Something like: Adjust the concrete type assigned to `FloatingType` based on the number of bits of the fixed-point representation as the currently hard-coded `float` type cannot represent numbers with more than 24 bits. should provide more context > > Also add a static assertion that IEEE754 floating point types are used. You're actually checking that the fixed point number can be represented with a `double` as IEEE754 also defines binary128 and binary256 if I'm reading it right. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/ipa/libipa/fixedpoint.h | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/src/ipa/libipa/fixedpoint.h b/src/ipa/libipa/fixedpoint.h > index 66863075c6..0640605bee 100644 > --- a/src/ipa/libipa/fixedpoint.h > +++ b/src/ipa/libipa/fixedpoint.h > @@ -25,19 +25,20 @@ private: > static constexpr unsigned int bits = I + F; > static_assert(bits <= sizeof(UT) * 8, "FixedPointQTraits: too many bits for type UT"); > > - /* > - * If fixed point storage is required with more than 24 bits, consider > - * updating this implementation to use double-precision floating point. > - */ > - static_assert(bits <= 24, "Floating point precision may be insufficient for more than 24 bits"); > + /* IEEE754 binary64 can faithfully represent integers up to 2^53 */ > + static_assert(bits <= 53, "Floating point precision may be insufficient"); > > static constexpr UT bitMask = bits < sizeof(UT) * 8 > ? (UT{ 1 } << bits) - 1 > : ~UT{ 0 }; > > public: > + /* IEEE754 binary32 can faithfully represent integers up to 2^24 */ I wouldn't mention standards that one has to go and learn about but simply say /* Use double for fixed point numbers with more than 24 bits. */ > using QuantizedType = UT; > - using FloatingType = float; > + using FloatingType = std::conditional_t<bits <= 24, float, double>; > + > + static_assert(std::numeric_limits<FloatingType>::is_iec559, > + "requires IEEE754 floating point"); as you define FloatingType one line above, I don't see how this can't be true. Also, ::is_iec559 returns true (usually, according to cppreference) for long double which are not supported > > static constexpr UT qMin = std::is_signed_v<T> > ? -(UT{ 1 } << (bits - 1)) > @@ -88,7 +89,7 @@ namespace details { > template<unsigned int Bits> > constexpr auto qtype() > { > - static_assert(Bits <= 32, > + static_assert(Bits <= 64, > "Unsupported number of bits for quantized type"); > > if constexpr (Bits <= 8) > @@ -97,6 +98,8 @@ constexpr auto qtype() > return int16_t(); > else if constexpr (Bits <= 32) > return int32_t(); > + else if constexpr (Bits <= 64) > + return int64_t(); > } > > } /* namespace details */ > -- > 2.55.0 >
2026. 09. 23. 16:55 keltezéssel, Jacopo Mondi írta: > Hi Barnabás > > On Fri, Sep 18, 2026 at 02:09:31PM +0200, Barnabás Pőcze wrote: >> If more than 24 bits would be used, use `double` as not all integers above >> 2^24 can be exactly represented by an IEEE754 binary32. > > Something like: > > Adjust the concrete type assigned to `FloatingType` based on the > number of bits of the fixed-point representation as the currently > hard-coded `float` type cannot represent numbers with more than 24 > bits. > > should provide more context Adjusted. > >> >> Also add a static assertion that IEEE754 floating point types are used. > > You're actually checking that the fixed point number can be > represented with a `double` as IEEE754 also defines binary128 and > binary256 if I'm reading it right. Well, technically, the `std::numeric_limits<FloatingType>::is_iec559` check is not sufficient to ensure that `double` is at least binary64 and `float` is at least binary32. I'll try to see if it can easily be improved. > >> >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> src/ipa/libipa/fixedpoint.h | 17 ++++++++++------- >> 1 file changed, 10 insertions(+), 7 deletions(-) >> >> diff --git a/src/ipa/libipa/fixedpoint.h b/src/ipa/libipa/fixedpoint.h >> index 66863075c6..0640605bee 100644 >> --- a/src/ipa/libipa/fixedpoint.h >> +++ b/src/ipa/libipa/fixedpoint.h >> @@ -25,19 +25,20 @@ private: >> static constexpr unsigned int bits = I + F; >> static_assert(bits <= sizeof(UT) * 8, "FixedPointQTraits: too many bits for type UT"); >> >> - /* >> - * If fixed point storage is required with more than 24 bits, consider >> - * updating this implementation to use double-precision floating point. >> - */ >> - static_assert(bits <= 24, "Floating point precision may be insufficient for more than 24 bits"); >> + /* IEEE754 binary64 can faithfully represent integers up to 2^53 */ >> + static_assert(bits <= 53, "Floating point precision may be insufficient"); >> >> static constexpr UT bitMask = bits < sizeof(UT) * 8 >> ? (UT{ 1 } << bits) - 1 >> : ~UT{ 0 }; >> >> public: >> + /* IEEE754 binary32 can faithfully represent integers up to 2^24 */ > > I wouldn't mention standards that one has to go and learn about but > simply say > > /* Use double for fixed point numbers with more than 24 bits. */ > >> using QuantizedType = UT; >> - using FloatingType = float; >> + using FloatingType = std::conditional_t<bits <= 24, float, double>; >> + >> + static_assert(std::numeric_limits<FloatingType>::is_iec559, >> + "requires IEEE754 floating point"); > > as you define FloatingType one line above, I don't see how this can't > be true. > > Also, ::is_iec559 returns true (usually, according to cppreference) > for long double which are not supported > >> >> static constexpr UT qMin = std::is_signed_v<T> >> ? -(UT{ 1 } << (bits - 1)) >> @@ -88,7 +89,7 @@ namespace details { >> template<unsigned int Bits> >> constexpr auto qtype() >> { >> - static_assert(Bits <= 32, >> + static_assert(Bits <= 64, >> "Unsupported number of bits for quantized type"); >> >> if constexpr (Bits <= 8) >> @@ -97,6 +98,8 @@ constexpr auto qtype() >> return int16_t(); >> else if constexpr (Bits <= 32) >> return int32_t(); >> + else if constexpr (Bits <= 64) >> + return int64_t(); >> } >> >> } /* namespace details */ >> -- >> 2.55.0 >>
diff --git a/src/ipa/libipa/fixedpoint.h b/src/ipa/libipa/fixedpoint.h index 66863075c6..0640605bee 100644 --- a/src/ipa/libipa/fixedpoint.h +++ b/src/ipa/libipa/fixedpoint.h @@ -25,19 +25,20 @@ private: static constexpr unsigned int bits = I + F; static_assert(bits <= sizeof(UT) * 8, "FixedPointQTraits: too many bits for type UT"); - /* - * If fixed point storage is required with more than 24 bits, consider - * updating this implementation to use double-precision floating point. - */ - static_assert(bits <= 24, "Floating point precision may be insufficient for more than 24 bits"); + /* IEEE754 binary64 can faithfully represent integers up to 2^53 */ + static_assert(bits <= 53, "Floating point precision may be insufficient"); static constexpr UT bitMask = bits < sizeof(UT) * 8 ? (UT{ 1 } << bits) - 1 : ~UT{ 0 }; public: + /* IEEE754 binary32 can faithfully represent integers up to 2^24 */ using QuantizedType = UT; - using FloatingType = float; + using FloatingType = std::conditional_t<bits <= 24, float, double>; + + static_assert(std::numeric_limits<FloatingType>::is_iec559, + "requires IEEE754 floating point"); static constexpr UT qMin = std::is_signed_v<T> ? -(UT{ 1 } << (bits - 1)) @@ -88,7 +89,7 @@ namespace details { template<unsigned int Bits> constexpr auto qtype() { - static_assert(Bits <= 32, + static_assert(Bits <= 64, "Unsupported number of bits for quantized type"); if constexpr (Bits <= 8) @@ -97,6 +98,8 @@ constexpr auto qtype() return int16_t(); else if constexpr (Bits <= 32) return int32_t(); + else if constexpr (Bits <= 64) + return int64_t(); } } /* namespace details */
If more than 24 bits would be used, use `double` as not all integers above 2^24 can be exactly represented by an IEEE754 binary32. Also add a static assertion that IEEE754 floating point types are used. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/ipa/libipa/fixedpoint.h | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)