65-bit Addition Error

[johnmcfarlane]

From #447: https://godbolt.org/z/gre-SC

[johnmcfarlane]

@elcojacobs preliminary investigation suggests a 65-bit arithmetic operation. That first failing test will compile if you enable 128-bit integer. Presumably that's not an option for RPi builds.

[johnmcfarlane]

@elcojacobs confirmed this is because default for CNL_INT128 changed. You were previously relying on 128-bit integer. You can re-enable it with #define CNL_USE_INT128=1.

[johnmcfarlane]

(I'm looking into why adding a couple of 32-bit numbers even needs 65 bits BTW. I may be able to address that.)

[elcojacobs]

I don't think I was using 128 bit. Even multiplying a 32 bit elastic number with a uint16 gives a compile error.

[johnmcfarlane]

There's an issue where if elastic_integer is wrapped in another type such as overflow_integer, it doesn't scale numbers efficiently. In this case, the scaling factor is 1 which should be a elastic_integer<1> but instead is elastic_integer<31>. Combined with the earlier addition, this pushes the arithmetic operation over the threshold into 65 bits (64 digits plus sign bit).

Can I ask if you're sure you want/need overflow_integer<R, saturated_overflow_tag>? It was recently pointed out that it doesn't do what you might expect as once something is positively saturated, you can still subtract from it, divide it etc. to get a smaller value. It's the topic of #381 to provide something that behaves more like float.

[elcojacobs]

What I want from the type is:

• auto c = a * b results in c being twice the width of a and b.
• decltype(a) c = a * b saturates c to the maximum value that can be stored by the type
• auto c = a*b; b = c; results in saturating the new value of b in the assignment.

I guess it can be summarized by saying that I want saturation to happen whenever normal integer arithmetic would overflow. I don't care that if I reach the saturated value and then subtract from it again, that I don't know it has overflowed. Perhaps I do not need an elastic integer at all and should only rely on the return type of arithmetic operations for intermediate results to be 'elastic' already because it returns a new type?

By wrapping the elastic_integer in an overflow integer I thought that I would stretch the type if I could and saturate if I could not.

[johnmcfarlane]

Yes, that's correct. You're using the library as intended but the implementation has inefficiencies which I need to address. The correct solution may take some time to figure out though. Please bear with me.

On Wed, 26 Jun 2019 at 13:11, Elco Jacobs notifications@github.com wrote:

What I want from the type is:

• auto c = a * b results in c being twice the width of a and b.
• decltype(a) c = a * b saturates c to the maximum value that can be stored by the type
• auto c = a*b; b = c; results in saturating the new value of b in the assignment.

I guess it can be summarized by saying that I want saturation to happen whenever normal integer arithmetic would overflow. I don't care that if I reach the saturated value and then subtract from it again, that I don't know it has overflowed. Perhaps I do not need an elastic integer at all and should only rely on the return type of arithmetic operations for intermediate results to be 'elastic' already because it returns a new type?

By wrapping the elastic_integer in an overflow integer I thought that I would stretch the type if I could and saturate if I could not.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/johnmcfarlane/cnl/issues/470?email_source=notifications&email_token=AAFTGN7EXKKTWKLLIEHCFTTP4NMHNA5CNFSM4H2XAYB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYTKBDA#issuecomment-505847948, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFTGN2FAOMKFC5IN5TQK6TP4NMHNANCNFSM4H2XAYBQ .

[johnmcfarlane]

Note to self: test written in Jun 2019 (!) to capture this:

#if !defined(CNL_UNREACHABLE_UB_ENABLED)
    TEST(safe_integer, construction)
    {
        cnl::safe_integer<31> ex{987653210LL};
        cnl::safe_integer<32> a{987653210LL};
        cnl::safe_integer<31> ac{cnl::_impl::scale<0, 2>(a)};
        ASSERT_TRUE(identical(ex, ac));
    }
#endif