[libc-commits] [PATCH] D132184: [libc] add division to UInt

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
Comment at: libc/src/__support/CPP/UInt.h:236
+  // cannot easily evaluate its overflow, so it returns true if it has any
+  // overflow, and relies on the client to use this information.
+  constexpr bool mul(const UInt<Bits> &other) {
----------------
sivachandra wrote:
> What is the use case of detecting the overflow?
I used it in my initial division implementation, but I don't have a use for it now, so I've removed it.


================
Comment at: libc/src/__support/CPP/UInt.h:293
+    }
+    if (other == UInt<Bits>(1)) {
+      return remainder;
----------------
sivachandra wrote:
> Should we implement `operator==(uint64_t)` to make this cleaner like: `other == uint64_t(1)`?
It appears that's unnecessary. I just assumed it'd need the explicit typing, but it will apparently get promoted implicitly just fine.


================
Comment at: libc/src/__support/CPP/UInt.h:338
+      } else {
+        // TODO(michaelrj): use builtins?
+        uint64_t num = val[i - 1];
----------------
sivachandra wrote:
> Yes - we have https://github.com/llvm/llvm-project/blob/main/libc/src/__support/FPUtil/builtin_wrappers.h.
that's in FPUtil, which depends on UInt. I will move that out in a followup patch, since it doesn't need to be in FPUtil.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132184