diff mbox series

[v3,05/22] host-utils: move checks out of divu128/divs128

Message ID 20210910112624.72748-6-luis.pires@eldorado.org.br
State New
Headers show
Series target/ppc: DFP instructions using decodetree | expand

Commit Message

Luis Fernando Fujita Pires Sept. 10, 2021, 11:26 a.m. UTC
In preparation for changing the divu128/divs128 implementations
to allow for quotients larger than 64 bits, move the div-by-zero
and overflow checks to the callers.

Signed-off-by: Luis Pires <luis.pires@eldorado.org.br>
---
 include/hw/clock.h        |  5 +++--
 include/qemu/host-utils.h | 36 +++++++++++++----------------------
 target/ppc/int_helper.c   | 14 +++++++++-----
 util/host-utils.c         | 40 ++++++++++++++++++---------------------
 4 files changed, 43 insertions(+), 52 deletions(-)

Comments

Richard Henderson Oct. 21, 2021, 8:11 p.m. UTC | #1
On 9/10/21 4:26 AM, Luis Pires wrote:
> In preparation for changing the divu128/divs128 implementations
> to allow for quotients larger than 64 bits, move the div-by-zero
> and overflow checks to the callers.
> 
> Signed-off-by: Luis Pires <luis.pires@eldorado.org.br>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Frederic, I had forgotten about this work that Luis has done here and in the next 3 
patches.  This overlaps with what you were doing in the RV128 patch set wrt getting a 
proper 128-bit divide.


r~

> ---
>   include/hw/clock.h        |  5 +++--
>   include/qemu/host-utils.h | 36 +++++++++++++----------------------
>   target/ppc/int_helper.c   | 14 +++++++++-----
>   util/host-utils.c         | 40 ++++++++++++++++++---------------------
>   4 files changed, 43 insertions(+), 52 deletions(-)
> 
> diff --git a/include/hw/clock.h b/include/hw/clock.h
> index a7187eab95..5a40a076aa 100644
> --- a/include/hw/clock.h
> +++ b/include/hw/clock.h
> @@ -320,8 +320,9 @@ static inline uint64_t clock_ns_to_ticks(const Clock *clk, uint64_t ns)
>           return 0;
>       }
>       /*
> -     * Ignore divu128() return value as we've caught div-by-zero and don't
> -     * need different behaviour for overflow.
> +     * BUG: when CONFIG_INT128 is not defined, the current implementation of
> +     * divu128 does not return a valid truncated quotient, so the result will
> +     * be wrong.
>        */
>       divu128(&lo, &hi, clk->period);
>       return lo;
> diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
> index ca9f3f021b..e82e6239af 100644
> --- a/include/qemu/host-utils.h
> +++ b/include/qemu/host-utils.h
> @@ -52,36 +52,26 @@ static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
>       return (__int128_t)a * b / c;
>   }
>   
> -static inline int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor)
> -{
> -    if (divisor == 0) {
> -        return 1;
> -    } else {
> -        __uint128_t dividend = ((__uint128_t)*phigh << 64) | *plow;
> -        __uint128_t result = dividend / divisor;
> -        *plow = result;
> -        *phigh = dividend % divisor;
> -        return result > UINT64_MAX;
> -    }
> +static inline void divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor)
> +{
> +    __uint128_t dividend = ((__uint128_t)*phigh << 64) | *plow;
> +    __uint128_t result = dividend / divisor;
> +    *plow = result;
> +    *phigh = dividend % divisor;
>   }
>   
> -static inline int divs128(int64_t *plow, int64_t *phigh, int64_t divisor)
> +static inline void divs128(int64_t *plow, int64_t *phigh, int64_t divisor)
>   {
> -    if (divisor == 0) {
> -        return 1;
> -    } else {
> -        __int128_t dividend = ((__int128_t)*phigh << 64) | (uint64_t)*plow;
> -        __int128_t result = dividend / divisor;
> -        *plow = result;
> -        *phigh = dividend % divisor;
> -        return result != *plow;
> -    }
> +    __int128_t dividend = ((__int128_t)*phigh << 64) | (uint64_t)*plow;
> +    __int128_t result = dividend / divisor;
> +    *plow = result;
> +    *phigh = dividend % divisor;
>   }
>   #else
>   void muls64(uint64_t *plow, uint64_t *phigh, int64_t a, int64_t b);
>   void mulu64(uint64_t *plow, uint64_t *phigh, uint64_t a, uint64_t b);
> -int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor);
> -int divs128(int64_t *plow, int64_t *phigh, int64_t divisor);
> +void divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor);
> +void divs128(int64_t *plow, int64_t *phigh, int64_t divisor);
>   
>   static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
>   {
> diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
> index f5dac3aa87..510faf24cf 100644
> --- a/target/ppc/int_helper.c
> +++ b/target/ppc/int_helper.c
> @@ -104,10 +104,11 @@ uint64_t helper_divdeu(CPUPPCState *env, uint64_t ra, uint64_t rb, uint32_t oe)
>       uint64_t rt = 0;
>       int overflow = 0;
>   
> -    overflow = divu128(&rt, &ra, rb);
> -
> -    if (unlikely(overflow)) {
> +    if (unlikely(rb == 0 || ra >= rb)) {
> +        overflow = 1;
>           rt = 0; /* Undefined */
> +    } else {
> +        divu128(&rt, &ra, rb);
>       }
>   
>       if (oe) {
> @@ -122,10 +123,13 @@ uint64_t helper_divde(CPUPPCState *env, uint64_t rau, uint64_t rbu, uint32_t oe)
>       int64_t rt = 0;
>       int64_t ra = (int64_t)rau;
>       int64_t rb = (int64_t)rbu;
> -    int overflow = divs128(&rt, &ra, rb);
> +    int overflow = 0;
>   
> -    if (unlikely(overflow)) {
> +    if (unlikely(rb == 0 || uabs64(ra) >= uabs64(rb))) {
> +        overflow = 1;
>           rt = 0; /* Undefined */
> +    } else {
> +        divs128(&rt, &ra, rb);
>       }
>   
>       if (oe) {
> diff --git a/util/host-utils.c b/util/host-utils.c
> index a789a11b46..701a371843 100644
> --- a/util/host-utils.c
> +++ b/util/host-utils.c
> @@ -86,24 +86,23 @@ void muls64 (uint64_t *plow, uint64_t *phigh, int64_t a, int64_t b)
>       *phigh = rh;
>   }
>   
> -/* Unsigned 128x64 division.  Returns 1 if overflow (divide by zero or */
> -/* quotient exceeds 64 bits).  Otherwise returns quotient via plow and */
> -/* remainder via phigh. */
> -int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor)
> +/*
> + * Unsigned 128-by-64 division. Returns quotient via plow and
> + * remainder via phigh.
> + * The result must fit in 64 bits (plow) - otherwise, the result
> + * is undefined.
> + * This function will cause a division by zero if passed a zero divisor.
> + */
> +void divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor)
>   {
>       uint64_t dhi = *phigh;
>       uint64_t dlo = *plow;
>       unsigned i;
>       uint64_t carry = 0;
>   
> -    if (divisor == 0) {
> -        return 1;
> -    } else if (dhi == 0) {
> +    if (divisor == 0 || dhi == 0) {
>           *plow  = dlo / divisor;
>           *phigh = dlo % divisor;
> -        return 0;
> -    } else if (dhi >= divisor) {
> -        return 1;
>       } else {
>   
>           for (i = 0; i < 64; i++) {
> @@ -120,15 +119,20 @@ int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor)
>   
>           *plow = dlo;
>           *phigh = dhi;
> -        return 0;
>       }
>   }
>   
> -int divs128(int64_t *plow, int64_t *phigh, int64_t divisor)
> +/*
> + * Signed 128-by-64 division. Returns quotient via plow and
> + * remainder via phigh.
> + * The result must fit in 64 bits (plow) - otherwise, the result
> + * is undefined.
> + * This function will cause a division by zero if passed a zero divisor.
> + */
> +void divs128(int64_t *plow, int64_t *phigh, int64_t divisor)
>   {
>       int sgn_dvdnd = *phigh < 0;
>       int sgn_divsr = divisor < 0;
> -    int overflow = 0;
>   
>       if (sgn_dvdnd) {
>           *plow = ~(*plow);
> @@ -145,19 +149,11 @@ int divs128(int64_t *plow, int64_t *phigh, int64_t divisor)
>           divisor = 0 - divisor;
>       }
>   
> -    overflow = divu128((uint64_t *)plow, (uint64_t *)phigh, (uint64_t)divisor);
> +    divu128((uint64_t *)plow, (uint64_t *)phigh, (uint64_t)divisor);
>   
>       if (sgn_dvdnd  ^ sgn_divsr) {
>           *plow = 0 - *plow;
>       }
> -
> -    if (!overflow) {
> -        if ((*plow < 0) ^ (sgn_dvdnd ^ sgn_divsr)) {
> -            overflow = 1;
> -        }
> -    }
> -
> -    return overflow;
>   }
>   #endif
>   
>
diff mbox series

Patch

diff --git a/include/hw/clock.h b/include/hw/clock.h
index a7187eab95..5a40a076aa 100644
--- a/include/hw/clock.h
+++ b/include/hw/clock.h
@@ -320,8 +320,9 @@  static inline uint64_t clock_ns_to_ticks(const Clock *clk, uint64_t ns)
         return 0;
     }
     /*
-     * Ignore divu128() return value as we've caught div-by-zero and don't
-     * need different behaviour for overflow.
+     * BUG: when CONFIG_INT128 is not defined, the current implementation of
+     * divu128 does not return a valid truncated quotient, so the result will
+     * be wrong.
      */
     divu128(&lo, &hi, clk->period);
     return lo;
diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h
index ca9f3f021b..e82e6239af 100644
--- a/include/qemu/host-utils.h
+++ b/include/qemu/host-utils.h
@@ -52,36 +52,26 @@  static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
     return (__int128_t)a * b / c;
 }
 
-static inline int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor)
-{
-    if (divisor == 0) {
-        return 1;
-    } else {
-        __uint128_t dividend = ((__uint128_t)*phigh << 64) | *plow;
-        __uint128_t result = dividend / divisor;
-        *plow = result;
-        *phigh = dividend % divisor;
-        return result > UINT64_MAX;
-    }
+static inline void divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor)
+{
+    __uint128_t dividend = ((__uint128_t)*phigh << 64) | *plow;
+    __uint128_t result = dividend / divisor;
+    *plow = result;
+    *phigh = dividend % divisor;
 }
 
-static inline int divs128(int64_t *plow, int64_t *phigh, int64_t divisor)
+static inline void divs128(int64_t *plow, int64_t *phigh, int64_t divisor)
 {
-    if (divisor == 0) {
-        return 1;
-    } else {
-        __int128_t dividend = ((__int128_t)*phigh << 64) | (uint64_t)*plow;
-        __int128_t result = dividend / divisor;
-        *plow = result;
-        *phigh = dividend % divisor;
-        return result != *plow;
-    }
+    __int128_t dividend = ((__int128_t)*phigh << 64) | (uint64_t)*plow;
+    __int128_t result = dividend / divisor;
+    *plow = result;
+    *phigh = dividend % divisor;
 }
 #else
 void muls64(uint64_t *plow, uint64_t *phigh, int64_t a, int64_t b);
 void mulu64(uint64_t *plow, uint64_t *phigh, uint64_t a, uint64_t b);
-int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor);
-int divs128(int64_t *plow, int64_t *phigh, int64_t divisor);
+void divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor);
+void divs128(int64_t *plow, int64_t *phigh, int64_t divisor);
 
 static inline uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
 {
diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
index f5dac3aa87..510faf24cf 100644
--- a/target/ppc/int_helper.c
+++ b/target/ppc/int_helper.c
@@ -104,10 +104,11 @@  uint64_t helper_divdeu(CPUPPCState *env, uint64_t ra, uint64_t rb, uint32_t oe)
     uint64_t rt = 0;
     int overflow = 0;
 
-    overflow = divu128(&rt, &ra, rb);
-
-    if (unlikely(overflow)) {
+    if (unlikely(rb == 0 || ra >= rb)) {
+        overflow = 1;
         rt = 0; /* Undefined */
+    } else {
+        divu128(&rt, &ra, rb);
     }
 
     if (oe) {
@@ -122,10 +123,13 @@  uint64_t helper_divde(CPUPPCState *env, uint64_t rau, uint64_t rbu, uint32_t oe)
     int64_t rt = 0;
     int64_t ra = (int64_t)rau;
     int64_t rb = (int64_t)rbu;
-    int overflow = divs128(&rt, &ra, rb);
+    int overflow = 0;
 
-    if (unlikely(overflow)) {
+    if (unlikely(rb == 0 || uabs64(ra) >= uabs64(rb))) {
+        overflow = 1;
         rt = 0; /* Undefined */
+    } else {
+        divs128(&rt, &ra, rb);
     }
 
     if (oe) {
diff --git a/util/host-utils.c b/util/host-utils.c
index a789a11b46..701a371843 100644
--- a/util/host-utils.c
+++ b/util/host-utils.c
@@ -86,24 +86,23 @@  void muls64 (uint64_t *plow, uint64_t *phigh, int64_t a, int64_t b)
     *phigh = rh;
 }
 
-/* Unsigned 128x64 division.  Returns 1 if overflow (divide by zero or */
-/* quotient exceeds 64 bits).  Otherwise returns quotient via plow and */
-/* remainder via phigh. */
-int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor)
+/*
+ * Unsigned 128-by-64 division. Returns quotient via plow and
+ * remainder via phigh.
+ * The result must fit in 64 bits (plow) - otherwise, the result
+ * is undefined.
+ * This function will cause a division by zero if passed a zero divisor.
+ */
+void divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor)
 {
     uint64_t dhi = *phigh;
     uint64_t dlo = *plow;
     unsigned i;
     uint64_t carry = 0;
 
-    if (divisor == 0) {
-        return 1;
-    } else if (dhi == 0) {
+    if (divisor == 0 || dhi == 0) {
         *plow  = dlo / divisor;
         *phigh = dlo % divisor;
-        return 0;
-    } else if (dhi >= divisor) {
-        return 1;
     } else {
 
         for (i = 0; i < 64; i++) {
@@ -120,15 +119,20 @@  int divu128(uint64_t *plow, uint64_t *phigh, uint64_t divisor)
 
         *plow = dlo;
         *phigh = dhi;
-        return 0;
     }
 }
 
-int divs128(int64_t *plow, int64_t *phigh, int64_t divisor)
+/*
+ * Signed 128-by-64 division. Returns quotient via plow and
+ * remainder via phigh.
+ * The result must fit in 64 bits (plow) - otherwise, the result
+ * is undefined.
+ * This function will cause a division by zero if passed a zero divisor.
+ */
+void divs128(int64_t *plow, int64_t *phigh, int64_t divisor)
 {
     int sgn_dvdnd = *phigh < 0;
     int sgn_divsr = divisor < 0;
-    int overflow = 0;
 
     if (sgn_dvdnd) {
         *plow = ~(*plow);
@@ -145,19 +149,11 @@  int divs128(int64_t *plow, int64_t *phigh, int64_t divisor)
         divisor = 0 - divisor;
     }
 
-    overflow = divu128((uint64_t *)plow, (uint64_t *)phigh, (uint64_t)divisor);
+    divu128((uint64_t *)plow, (uint64_t *)phigh, (uint64_t)divisor);
 
     if (sgn_dvdnd  ^ sgn_divsr) {
         *plow = 0 - *plow;
     }
-
-    if (!overflow) {
-        if ((*plow < 0) ^ (sgn_dvdnd ^ sgn_divsr)) {
-            overflow = 1;
-        }
-    }
-
-    return overflow;
 }
 #endif