[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 03/21] Int128.h: addition of a few 128-bit operations
|
From: |
Richard Henderson |
|
Subject: |
Re: [PATCH v3 03/21] Int128.h: addition of a few 128-bit operations |
|
Date: |
Tue, 19 Oct 2021 11:15:20 -0700 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 |
On 10/19/21 2:47 AM, Frédéric Pétrot wrote:
+static inline void divrem128(uint64_t ul, uint64_t uh,
+ uint64_t vl, uint64_t vh,
+ uint64_t *ql, uint64_t *qh,
+ uint64_t *rl, uint64_t *rh)
I think we should move all of the division implementation out of the header; this is
really much too large to inline.
I think util/int128.c would be a reasonable place.
That said, why are you splitting the Int128 apart to pass as pieces here? Seems like
passing the Int128 and doing the split inside would make more sense.
+ /* never happens, but makes gcc shy */
+ n = 0;
Then g_assert_not_reached(), or change the previous if to an assert.
Hmm, it's not "never happens" so much as "divide by zero".
Please update the comment accordingly.
+ if (r != NULL) {
+ r[0] = k;
+ }
r is a local array; useless check for null.
+ s = clz32(v[n - 1]); /* 0 <= s <= 32 */
+ if (s != 0) {
+ for (i = n - 1; i > 0; i--) {
+ vn[i] = ((v[i] << s) | (v[i - 1] >> (32 - s)));
+ }
+ vn[0] = v[0] << s;
+
+ un[m] = u[m - 1] >> (32 - s);
+ for (i = m - 1; i > 0; i--) {
+ un[i] = (u[i] << s) | (u[i - 1] >> (32 - s));
+ }
+ un[0] = u[0] << s;
Why are you shifting the 128-bit value in 4 parts, rather than letting int128_lshift do
the job?
r~
- [PATCH v3 00/21] Adding partial support for 128-bit riscv target, Frédéric Pétrot, 2021/10/19
- [PATCH v3 02/21] memory: add a few defines for octo (128-bit) values, Frédéric Pétrot, 2021/10/19
- [PATCH v3 04/21] target/riscv: additional macros to check instruction support, Frédéric Pétrot, 2021/10/19
- [PATCH v3 03/21] Int128.h: addition of a few 128-bit operations, Frédéric Pétrot, 2021/10/19
- Re: [PATCH v3 03/21] Int128.h: addition of a few 128-bit operations,
Richard Henderson <=
- [PATCH v3 06/21] target/riscv: array for the 64 upper bits of 128-bit registers, Frédéric Pétrot, 2021/10/19
- [PATCH v3 09/21] target/riscv: moving some insns close to similar insns, Frédéric Pétrot, 2021/10/19
- [PATCH v3 05/21] target/riscv: separation of bitwise logic and aritmetic helpers, Frédéric Pétrot, 2021/10/19
- [PATCH v3 01/21] memory: change define name for consistency, Frédéric Pétrot, 2021/10/19
- [PATCH v3 08/21] target/riscv: adding accessors to the registers upper part, Frédéric Pétrot, 2021/10/19