qemu-ppc
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Richard Henderson
Subject: Re: [PATCH v3 05/22] host-utils: move checks out of divu128/divs128
Date: Thu, 21 Oct 2021 13:11:35 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 
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
reply via email to
[Prev in Thread] Current Thread [Next in Thread]