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

Tue Ly via Phabricator via libc-commits libc-commits at lists.llvm.org
Wed May 4 15:34:30 PDT 2022

lntue added inline comments.

Comment at: libc/src/__support/UInt128.h:41
+  UInt128 operator+(const UInt128 other) {
+    uint64_t new_low, new_high;
`other` should either be `UInt128` or `const UInt128&`.  You can also add `const` after the declaration:
`UInt128 operator+(UInt128 other) const {`.  Same for `operator*`.

Comment at: libc/src/__support/UInt128.h:80
+  UInt128 operator<<(const int amount) {
+    uint64_t new_low = low << amount;
Drop the `const` for `amount` and add `const` at the end:
`UInt128 operator<<(int amount) const {`.  Same for other operators.

Comment at: libc/src/__support/UInt128.h:87
+  UInt128 operator>>(const int amount) {
+    uint64_t new_low = (low >> amount) | (high << (BITS_IN_UINT64 - amount));
+    uint64_t new_high = (high >> amount);
`BITS_IN_UINT64` is `unsigned`, mixing with signed `amount` might trigger UB / warnings.

Comment at: libc/test/src/__support/uint128_test.cpp:13
+// while I could use "ASSERT_TRUE(a == b)", this will tell me what the values
+// actually are.
You should be able to add specialization for UInt128 in `LibcTest.cpp` and use ASSERT_EQ, EXPECT_EQ as usual.  Also disable the `__uint128_t` variant over there when it's not defined.

  rG LLVM Github Monorepo



More information about the libc-commits mailing list