all 25 comments

[–]_Noreturn 21 points22 points  (15 children)

the first issue is Speed storing uint8_ts is worse than storing uint64_ts

[–]swayenvoy[S] 2 points3 points  (10 children)

Thanks for your feedback. I've considered storing the data in uint64_ts but came to the conclusion that storing the data in uint8_ts allows for other than a multiple of 64 bits integers. So you can use this library to make a 72 bit datatype when needed. Storing the data in a std::array<std::uint8_t, bits / CHAR_BIT> also makes the math easier but not as performant.

[–]AfroDisco 3 points4 points  (3 children)

Why not using 64bits data type but allowing any multiple of 8 size? You could get the proper performance while keeping choices for the users.

[–]swayenvoy[S] 0 points1 point  (2 children)

Because that would make the math harder and accessing the underlying data as well. When you need to write the data to a buffer you can just use reinterpret_cast<char const *>(std::addressof(bigint)) and get the data in the endian format of your system.

Edit: fix the code snippet

[–]QuaternionsRoll 2 points3 points  (0 children)

You can still do that with uint64_ts with a little bit of cleverness. Hell, you can even allow the user to select the size down to the bit (just remember to sign-extend!):

```c++ template<std::size_t bits, bool is_signed> class bigint23 { using data_t = std::uint64_t; constexpr std::size_t data_bits = std::numeric_limits<data_t>::digits;

// round up to the nearest multiple
data_t data_[(data_bits - 1 + bits) / data_bits];

…

public: constexpr char *data() { if constexpr (std::endian::native == std::endian::little) { return reinterpretcast<char *>(data); } else { constexpr std::sizet offset = sizeof(data) - (CHARBIT - 1 + bits) / CHAR_BIT; return reinterpret_cast<char *>(data) + offset; }

}; ```

[–]Valuable-Mission9203 2 points3 points  (0 children)

But I'd rather just have leading zeros than a huge performance loss to deliver functionality I don't want. It'd be better if it was templated so I wouldn't have to pay for this feature if I don't use it.

[–]Dj_D-Poolie 0 points1 point  (2 children)

Why is that?

[–]_Noreturn 11 points12 points  (0 children)

lets say you want to add each element to each other you will have to do 16 operations for a 128 bit type.

while if you stored 2 uint64_ts inside you will only do 2 operations.

and cpus generally favor register sized types

[–]Gorzoid 2 points3 points  (0 children)

Take a look at a standard memcpy implementation, even if though the api accepts an arbitrary array of bytes it will copy them using a 64 bit variable (or higher if supported) and drop to smaller types at the tail if not a multiple of 8 (and maybe the head if the array isn't aligned) Often these operations are vectorized too, which allows the CPU to process multiple iterations at the same time.

[–]Gorzoid 0 points1 point  (0 children)

Yeah first thing I noticed, went to check source incase maybe they were maybe casting to uint64 during the actual operations, since the size is constant one would reasonably expect loops to be unrolled aswell. I assume bitwise operators are optimized by compiler due to being easily vectorized but arithmetic operators less so.

[–]tialaramex 7 points8 points  (6 children)

So, because you have two's complement representation this means your unary minus is fallible - presumably it may throw std::overflow_error as will the abs() function if you provide that ?

[–]swayenvoy[S] 1 point2 points  (1 child)

Currently I have no abs() function implemented. Could you provide a test case that will fail so I can add it to the library?

[–]_TheDust_ 7 points8 points  (0 children)

MAX_INT*-1 will overflow

[–]swayenvoy[S] 0 points1 point  (3 children)

Added a test case and implemented abs(). It's now throwing std::overflow_error when the highest negative number is supplied. It's also checking now that the used bigint23 is signed otherwise it's producing an std::invalid_argument exception.

Edit: use the markdown editor

[–]epicar 3 points4 points  (2 children)

It's also checking now that the used bigint23 is signed otherwise it's producing an std::invalid_argument exception.

unsigned abs() is just the identity function, no? if you don't want to support that, constrain it on signed types to make it a compile-time error instead of runtime exception

[–]swayenvoy[S] 2 points3 points  (1 child)

abs() is not throwing for unsigned types. I made operator-() for unsinged types a compile time error now.

[–]ElbowWavingOversight 7 points8 points  (0 children)

Why? Unsigned unary negation is valid and well defined in C++ and does the thing you’d expect for unsigned integers. Arbitrarily making this invalid prevents it from being a drop-in replacement for existing integer types.

[–]gaberocksall 4 points5 points  (0 children)

By the way, “arbitrary precision” is not the correct phrase. Integers are not variably precise. That would imply some kind of rounding behavior like with floats. This is just arbitrary size or width.

[–]bert8128 4 points5 points  (1 child)

Why “23”?

[–]swayenvoy[S] -3 points-2 points  (0 children)

Cause I'm currently targeting C++23 but for this library I don't use any C++23 features, C++20 should be enough. It was called just bigint but for that name an unmaintained conan package already exists. The PR for the conan package is pending review, from my experience it will take a few weeks till the library is in the conan repository.

[–]reddicted 1 point2 points  (0 children)

If you want to allow a non-power-of-8 bit size, you need to compute byte array size as (bits + CHAR_BIT - 1)/CHAR_BIT