[libc-commits] [PATCH] D124959: [libc] add uint128 implementation

[Michael Jones via Phabricator via libc-commits]

michaelrj added a comment.

I'm not sure that extending `UInt.h` is a good idea, since it adds a lot of complexity for not a lot of benefit. If we had actual uses for integers larger than 128 bits, then I would agree, but right now the `UInt` class is not used in any entrypoint. There is a use of 192 bit integers in `src/__support/FPUtil/XFloat.h` which is included in `src/math/generic/dp_trig.cpp`, but that's never actually used. To move these functions to `UInt` would involve rewriting all of these functions to support arbitrary sized integers when in reality there's only one size used. This would be easy for most things, but would be very difficult for multiplying two 128 bit integers. Finally, The designs of these classes are different, my `UInt128` class is intended to be an immutable number, acting similar to a primitive. The `UInt` class acts more like a proper class, its functions take an argument and modify the object instead of creating a new object to be returned. These are both valid designs, but mine is closer to what's expected when using `__uint128_t`, which is I think what's most important.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124959/new/

https://reviews.llvm.org/D124959