[v2,06/27] int128: Use complex numbers if advisable

Commit Message

Richard Henderson July 1, 2016, 5:04 p.m. UTC

If __int128 is not supported, prefer a base type that is
returned in registers rather than memory.

Signed-off-by: Richard Henderson <[e-mail]>
---
 include/qemu/int128.h | 110 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 69 insertions(+), 41 deletions(-)

Comments

Paolo Bonzini July 4, 2016, 11:51 a.m. UTC | #1

On 01/07/2016 19:04, Richard Henderson wrote:
> If __int128 is not supported, prefer a base type that is
> returned in registers rather than memory.
> 
> Signed-off-by: Richard Henderson <[e-mail]>
> ---
>  include/qemu/int128.h | 110 +++++++++++++++++++++++++++++++-------------------
>  1 file changed, 69 insertions(+), 41 deletions(-)
> 
> diff --git a/include/qemu/int128.h b/include/qemu/int128.h
> index 67440fa..ab67275 100644
> --- a/include/qemu/int128.h
> +++ b/include/qemu/int128.h
> @@ -139,27 +139,37 @@ static inline void int128_subfrom(Int128 *a, Int128 b)
>  
>  #else /* !CONFIG_INT128 */
>  
> -typedef struct Int128 Int128;
> +/* Here we are catering to the ABI of the host.  If the host returns
> +   64-bit complex in registers, but the 128-bit structure in memory,
> +   then choose the complex representation.  */
> +#if defined(__GNUC__) \
> +    && (defined(__powerpc__) || defined(__sparc__)) \
> +    && !defined(CONFIG_TCG_INTERPRETER)
> +typedef _Complex unsigned long long Int128;

Is there any reason not to do that unconditionally?

Paolo

Peter Maydell July 4, 2016, 12:07 p.m. UTC | #2

On 1 July 2016 at 18:04, Richard Henderson <[e-mail]> wrote:
> If __int128 is not supported, prefer a base type that is
> returned in registers rather than memory.

So which host architectures does this improve?
128 bit integers are nothing to do with complex numbers,
so we ought to have a strong justification for abusing
the _Complex type.

The ifdef suggests this only helps ppc and sparc, which
to my mind is not a sufficient justification.

If there's much benefit from doing this then it would be
better for the compiler on those architectures to support
int128 as a proper native type returned in registers.

> +#if defined(__GNUC__) \
> +    && (defined(__powerpc__) || defined(__sparc__)) \
> +    && !defined(CONFIG_TCG_INTERPRETER)

Why the CONFIG_TCG_INTERPRETER clause ?

thanks
-- PMM

Patch

diff --git a/include/qemu/int128.h b/include/qemu/int128.h
index 67440fa..ab67275 100644
--- a/include/qemu/int128.h
+++ b/include/qemu/int128.h
@@ -139,27 +139,37 @@  static inline void int128_subfrom(Int128 *a, Int128 b)
 
 #else /* !CONFIG_INT128 */
 
-typedef struct Int128 Int128;
+/* Here we are catering to the ABI of the host.  If the host returns
+   64-bit complex in registers, but the 128-bit structure in memory,
+   then choose the complex representation.  */
+#if defined(__GNUC__) \
+    && (defined(__powerpc__) || defined(__sparc__)) \
+    && !defined(CONFIG_TCG_INTERPRETER)
+typedef _Complex unsigned long long Int128;
 
-struct Int128 {
-    uint64_t lo;
-    int64_t hi;
-};
+static inline Int128 int128_make128(uint64_t lo, uint64_t hi)
+{
+    return lo + 1i * hi;
+}
 
-static inline Int128 int128_make64(uint64_t a)
+static inline uint64_t int128_getlo(Int128 a)
 {
-    return (Int128) { a, 0 };
+    return __real__ a;
 }
 
-static inline Int128 int128_make128(uint64_t lo, uint64_t hi)
+static inline int64_t int128_gethi(Int128 a)
 {
-    return (Int128) { lo, hi };
+    return __imag__ a;
 }
+#else
+typedef struct Int128 {
+    uint64_t lo;
+    int64_t hi;
+} Int128;
 
-static inline uint64_t int128_get64(Int128 a)
+static inline Int128 int128_make128(uint64_t lo, uint64_t hi)
 {
-    assert(!a.hi);
-    return a.lo;
+    return (Int128) { lo, hi };
 }
 
 static inline uint64_t int128_getlo(Int128 a)
@@ -171,78 +181,92 @@  static inline int64_t int128_gethi(Int128 a)
 {
     return a.hi;
 }
+#endif /* complex 128 */
+
+static inline Int128 int128_make64(uint64_t a)
+{
+    return int128_make128(a, 0);
+}
+
+static inline uint64_t int128_get64(Int128 a)
+{
+    assert(int128_gethi(a) == 0);
+    return int128_getlo(a);
+}
 
 static inline Int128 int128_zero(void)
 {
-    return int128_make64(0);
+    return int128_make128(0, 0);
 }
 
 static inline Int128 int128_one(void)
 {
-    return int128_make64(1);
+    return int128_make128(1, 0);
 }
 
 static inline Int128 int128_2_64(void)
 {
-    return (Int128) { 0, 1 };
+    return int128_make128(0, 1);
 }
 
 static inline Int128 int128_exts64(int64_t a)
 {
-    return (Int128) { .lo = a, .hi = (a < 0) ? -1 : 0 };
+    return int128_make128(a, a < 0 ? -1 : 0);
 }
 
 static inline Int128 int128_and(Int128 a, Int128 b)
 {
-    return (Int128) { a.lo & b.lo, a.hi & b.hi };
+    uint64_t al = int128_getlo(a), bl = int128_getlo(b);
+    uint64_t ah = int128_gethi(a), bh = int128_gethi(b);
+
+    return int128_make128(al & bl, ah & bh);
 }
 
 static inline Int128 int128_rshift(Int128 a, int n)
 {
-    int64_t h;
-    if (!n) {
-        return a;
-    }
-    h = a.hi >> (n & 63);
-    if (n >= 64) {
+    uint64_t al = int128_getlo(a), ah = int128_gethi(a);
+    int64_t h = ((int64_t)ah) >> (n & 63);
+    if (n & 64) {
         return int128_make128(h, h >> 63);
     } else {
-        return int128_make128((a.lo >> n) | ((uint64_t)a.hi << (64 - n)), h);
+        return int128_make128((al >> n) | (ah << (64 - n)), h);
     }
 }
 
 static inline Int128 int128_add(Int128 a, Int128 b)
 {
-    uint64_t lo = a.lo + b.lo;
+    uint64_t al = int128_getlo(a), bl = int128_getlo(b);
+    uint64_t ah = int128_gethi(a), bh = int128_gethi(b);
+    uint64_t lo = al + bl;
 
-    /* a.lo <= a.lo + b.lo < a.lo + k (k is the base, 2^64).  Hence,
-     * a.lo + b.lo >= k implies 0 <= lo = a.lo + b.lo - k < a.lo.
-     * Similarly, a.lo + b.lo < k implies a.lo <= lo = a.lo + b.lo < k.
-     *
-     * So the carry is lo < a.lo.
-     */
-    return int128_make128(lo, (uint64_t)a.hi + b.hi + (lo < a.lo));
+    return int128_make128(lo, ah + bh + (lo < al));
 }
 
-static inline Int128 int128_neg(Int128 a)
+static inline Int128 int128_sub(Int128 a, Int128 b)
 {
-    uint64_t lo = -a.lo;
-    return int128_make128(lo, ~(uint64_t)a.hi + !lo);
+    uint64_t al = int128_getlo(a), bl = int128_getlo(b);
+    uint64_t ah = int128_gethi(a), bh = int128_gethi(b);
+
+    return int128_make128(al - bl, ah - bh - (al < bl));
 }
 
-static inline Int128 int128_sub(Int128 a, Int128 b)
+static inline Int128 int128_neg(Int128 a)
 {
-    return int128_make128(a.lo - b.lo, (uint64_t)a.hi - b.hi - (a.lo < b.lo));
+    uint64_t al = int128_getlo(a), ah = int128_gethi(a);
+    return int128_make128(-al, ~ah + !al);
 }
 
 static inline bool int128_nonneg(Int128 a)
 {
-    return a.hi >= 0;
+    return int128_gethi(a) >= 0;
 }
 
 static inline bool int128_eq(Int128 a, Int128 b)
 {
-    return a.lo == b.lo && a.hi == b.hi;
+    uint64_t al = int128_getlo(a), bl = int128_getlo(b);
+    uint64_t ah = int128_gethi(a), bh = int128_gethi(b);
+
+    return al == bl && ah == bh;
 }
 
 static inline bool int128_ne(Int128 a, Int128 b)
@@ -252,7 +276,10 @@  static inline bool int128_ne(Int128 a, Int128 b)
 
 static inline bool int128_ge(Int128 a, Int128 b)
 {
-    return a.hi > b.hi || (a.hi == b.hi && a.lo >= b.lo);
+    uint64_t al = int128_getlo(a), bl = int128_getlo(b);
+    int64_t  ah = int128_gethi(a), bh = int128_gethi(b);
+
+    return ah > bh || (ah == bh && al >= bl);
 }
 
 static inline bool int128_lt(Int128 a, Int128 b)
@@ -272,7 +299,8 @@  static inline bool int128_gt(Int128 a, Int128 b)
 
 static inline bool int128_nz(Int128 a)
 {
-    return a.lo || a.hi;
+    uint64_t al = int128_getlo(a), ah = int128_gethi(a);
+    return (al | ah) != 0;
 }
 
 static inline Int128 int128_min(Int128 a, Int128 b)