Re: Decimal: Formal review begins.

From: Matt Borland (matt_at_[hidden])
Date: 2025-01-16 01:02:20

> 

> > The name "decimal32" is occupied by the IEEE-754 compliant type. The
> > naming scheme matches published standards and existing practice such as
> > uint32_t vs uint_fast32_t. There's a clear statement of intent using the latter. I
> > don't think people should pick decimal32 over decimal32_fast in the general
> > case, but I also think it would be a bad idea to diverge from IEEE in our naming
> > scheme.
> 

> 

> Fair enough, N2849/TR 24733 do say that decimal32 occupies four octets.
> 

> (Although this gives significantly more guarantees in 24733 because you could
> bit_cast the standard decimal32 to uint32_t and observe the IEEE representation,
> whereas I don't think you can do that for this library's decimal32.)
> 

You can do that, and it's actually how I debugged the library in the infancy stage. You'll find tests against pre-defined bitsets I generated with pen and paper.

> However... even if we accept that the name decima32 belongs to the compact
> version, shouldn't there at least be a way to implicitly convert decimal32 to
> decimal32_fast? Shouldn't there be a way to (explicitly or implicitly) convert
> decimal32_fast to decimal32? Or at minimum, assign decimal32_fast to
> decimal32?
> 

> E.g.
> 

> void f( decimal32& x, decimal32& y )
> {
> decimal32_fast x2 = x;
> decimal32_fast y2 = y;
> // do calculations here using decimal32_fast
> x = <some result of type decimal32_fast>;
> 

> y = <some result of type decimal32_fast>;
> 

> }
> 

> Incidentally, the synopsis of decimal32_fast says
> 

> explicit constexpr operator decimal64() const noexcept;
> explicit constexpr operator decimal128() const noexcept;
> 

> but this doesn't seem correct to me. It also doesn't seem to correspond
> to what the source code says.
> 

> Furthermore, consider
> 

> void f( decimal32& x, decimal32& y )
> {
> decimal32_fast z = x + y;
> decimal32_fast w = x - y;
> // do calculations here using decimal32_fast
> x = <some result of type decimal32_fast>;
> 

> y = <some result of type decimal32_fast>;
> 

> }
> 

> These lines
> 

> decimal32_fast z = x + y;
> decimal32_fast w = x - y;
> 

> still perform an unnecessary pack and unpack. Shouldn't we be worried
> about that? Maybe a compiler can optimize this out, maybe it can't; I'll
> need to put Decimal on Compiler Explorer somehow to check.
> 

> This can be avoided by making operator+(decimal32, decimal32) return
> decimal32_fast. That's not unheard of because it's for instance what
> operator+(short, short) does, but it is not what TR 24733 specifies.

Every decimal type can be explicitly converted to any other decimal type. In the promotion system if you have an operation like decimal32 + decimal32_fast the result will be promoted to decimal32_fast since we considered it to be higher precedence (like how decimal32 + decimal64 yields decimal64). Right now the definition of BOOST_DECIMAL_DEC_EVAL_METHOD only allows internal promotion of width like FLT_EVAL_METHOD: https://en.cppreference.com/w/cpp/types/climits/FLT_EVAL_METHOD. What could be added is additional values that specify that if we have f(decimal32, decimal32) all internal calculations should be done with decimal32_fast. That would likely offer a decent speedup in the <cmath> implementations.

Matt