[PATCH] D120327: compiler-rt: Add udivmodei5 to builtins and add bitint library

[Freddy, Ye via Phabricator via llvm-commits]

FreddyYe added a comment.

To be honest, I'm not familiar in compiler-rt component. And my experience before was mainly to find out the feasibility to support arbitrary-bits-division for _BitInt(). Fortunately, my proposal was basically same as this one: emit a new libcall in backend; add library support; use malloc()... And I even used a more simple division algorithm model: sub-shift, which has been shared in the RFC link. So my experience before may not be very helpful here. Anyway I can try my best. So everyone is OK with use malloc(), right? That was my biggest concern. BTW FYI the largest _BitInt() support is 8,388,608 . Hi @mgehre-amd Will you upload another patch to set the bitint.a as a default link library of clang when this patch gets landed in? I believe FE will reopen _BitInt() > 128 bit unless division is supported by default but not by option `--rtlib=compiler-rt`. @erichkeane @aaron.ballman



================
Comment at: compiler-rt/lib/builtins/udivmodei5.c:41
+
+typedef unsigned int WordType;
+static const int WORD_SIZE_IN_BITS = sizeof(WordType) * CHAR_BIT;
----------------
Copy from Erich's comment: can this be uint64_t type?


================
Comment at: compiler-rt/lib/builtins/udivmodei5.c:46
+static int nlz16(uint16_t x) {
+  int n;
+
----------------
int n = 0;


================
Comment at: compiler-rt/lib/builtins/udivmodei5.c:100
+static int is_neg(const uint32_t *V, unsigned BitWidth) {
+  int words = getNumWords(BitWidth);
+  return V[words - 1] >> (WORD_SIZE_IN_BITS - 1);
----------------
unsigned


================
Comment at: compiler-rt/lib/builtins/udivmodei5.c:152
+  // b denotes the base of the number system. In our case b is 2^16.
+  const uint32_t b = (uint32_t)1 << 16;
+
----------------
Maybe it'd better not to include <stdint.h> but to use macros in int_types.h like other integer APIs:
```
typedef int32_t si_int;
typedef uint32_t su_int;
typedef int64_t di_int;
typedef uint64_t du_int;
```


 


================
Comment at: compiler-rt/lib/builtins/udivmodei5.c:432
+                    unsigned rhsWords, WordType *Quotient, WordType *Remainder,
+                    unsigned BitWidth) {
+  int LHSsign = is_neg(LHS, BitWidth);
----------------
NFC align naming convention: `BitWidth` and `bits`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120327