[Qemu-devel] [PATCH 0/5] Alpha improvments, round 3

[Qemu-devel] [PATCH 0/5] Alpha improvments, round 3

[Richard Henderson]

The first is the IMB patch, adjusted for Aurelien's comments.

The third is a bug in a generic header; I'm not sure what the
proper tagging is for that sort of thing.

The rest are new bugs found in the Alpha backend via the gcc
testsuite.  We're down to a handfull of failures over native
there.  I hope to post an approach to the rounding mode issue
shortly.


r~


Richard Henderson (5):
  target-alpha: Add placeholders for missing userspace PALcalls.
  target-alpha: Implement fp branch/cmov inline.
  target-alpha: Fix generic ctz64.
  target-alpha: Fix cvtlq.
  target-alpha: Fix float32_to_s vs zero exponent.

 host-utils.h             |    2 +-
 hw/alpha_palcode.c       |   32 +++++++-
 target-alpha/helper.h    |    7 --
 target-alpha/op_helper.c |   79 ++++++++----------
 target-alpha/translate.c |  197 ++++++++++++++++++++++++++--------------------
 5 files changed, 176 insertions(+), 141 deletions(-)

[Qemu-devel] [PATCH 1/5] target-alpha: Add placeholders for missing userspace PALcalls.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/alpha_palcode.c |   32 ++++++++++++++++++++++++++++++--
 1 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/hw/alpha_palcode.c b/hw/alpha_palcode.c
index edec018..2a694dc 100644
--- a/hw/alpha_palcode.c
+++ b/hw/alpha_palcode.c
@@ -1061,6 +1061,16 @@ void call_pal (CPUState *env, int palcode)
     target_long ret;
 
     switch (palcode) {
+    case 0x80:
+        /* BPT */
+        qemu_log("BPT\n");
+        /* FIXME: Sends SIGTRAP, si_code=TRAP_BRKPT.  */
+        exit(1);
+    case 0x81:
+        /* BUGCHK */
+        qemu_log("BUGCHK\n");
+        /* FIXME: Sends SIGTRAP, si_code=SI_FAULT.  */
+        exit(1);
     case 0x83:
         /* CALLSYS */
         qemu_log("CALLSYS n " TARGET_FMT_ld "\n", env->ir[0]);
@@ -1075,6 +1085,14 @@ void call_pal (CPUState *env, int palcode)
             env->ir[IR_V0] = -ret;
         }
         break;
+    case 0x86:
+        /* IMB */
+        qemu_log("IMB\n");
+	/* ??? We can probably elide the code using page_unprotect that is
+	   checking for self-modifying code.  Instead we could simply call
+	   tb_flush here.  Until we work out the changes required to turn
+	   off the extra write protection, this can be a no-op.  */
+        break;
     case 0x9E:
         /* RDUNIQUE */
         qemu_log("RDUNIQUE: " TARGET_FMT_lx "\n", env->unique);
@@ -1085,9 +1103,19 @@ void call_pal (CPUState *env, int palcode)
         qemu_log("WRUNIQUE: " TARGET_FMT_lx "\n", env->ir[IR_A0]);
         /* Handled in the translator for usermode.  */
         abort();
+    case 0xAA:
+        /* GENTRAP */
+        qemu_log("GENTRAP: " TARGET_FMT_lx "\n", env->ir[IR_A0]);
+        /* FIXME: This is supposed to send a signal:
+           SIGFPE:
+             GEN_INTOVF, GEN_INTDIV, GEN_FLTOVF, GEN_FLTDIV,
+             GEN_FLTUND, GEN_FLTINV, GEN_FLTINE, GEN_ROPRAND
+           SIGTRAP:
+             others
+           with various settings of si_code.  */
+        exit(1);
     default:
-        qemu_log("%s: unhandled palcode %02x\n",
-                    __func__, palcode);
+        qemu_log("%s: unhandled palcode %02x\n", __func__, palcode);
         exit(1);
     }
 }
-- 
1.6.5.2

[Qemu-devel] [PATCH 2/5] target-alpha: Implement fp branch/cmov inline.

[Richard Henderson]

The old fcmov implementation had a typo:
-        tcg_gen_mov_i64(cpu_fir[rc], cpu_fir[ra]);
which moved the condition, not the second source, to the destination.

But it's also easy to implement the simplified fp comparison inline.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-alpha/helper.h    |    7 --
 target-alpha/op_helper.c |   31 -------
 target-alpha/translate.c |  197 ++++++++++++++++++++++++++--------------------
 3 files changed, 110 insertions(+), 125 deletions(-)

diff --git a/target-alpha/helper.h b/target-alpha/helper.h
index 4eb3b6f..bedd3c0 100644
--- a/target-alpha/helper.h
+++ b/target-alpha/helper.h
@@ -77,13 +77,6 @@ DEF_HELPER_2(cmpgeq, i64, i64, i64)
 DEF_HELPER_2(cmpgle, i64, i64, i64)
 DEF_HELPER_2(cmpglt, i64, i64, i64)
 
-DEF_HELPER_1(cmpfeq, i64, i64)
-DEF_HELPER_1(cmpfne, i64, i64)
-DEF_HELPER_1(cmpflt, i64, i64)
-DEF_HELPER_1(cmpfle, i64, i64)
-DEF_HELPER_1(cmpfgt, i64, i64)
-DEF_HELPER_1(cmpfge, i64, i64)
-
 DEF_HELPER_2(cpys, i64, i64, i64)
 DEF_HELPER_2(cpysn, i64, i64, i64)
 DEF_HELPER_2(cpyse, i64, i64, i64)
diff --git a/target-alpha/op_helper.c b/target-alpha/op_helper.c
index d7f4fb2..8eba5ec 100644
--- a/target-alpha/op_helper.c
+++ b/target-alpha/op_helper.c
@@ -884,37 +884,6 @@ uint64_t helper_cmpglt(uint64_t a, uint64_t b)
         return 0;
 }
 
-uint64_t helper_cmpfeq (uint64_t a)
-{
-    return !(a & 0x7FFFFFFFFFFFFFFFULL);
-}
-
-uint64_t helper_cmpfne (uint64_t a)
-{
-    return (a & 0x7FFFFFFFFFFFFFFFULL);
-}
-
-uint64_t helper_cmpflt (uint64_t a)
-{
-    return (a & 0x8000000000000000ULL) && (a & 0x7FFFFFFFFFFFFFFFULL);
-}
-
-uint64_t helper_cmpfle (uint64_t a)
-{
-    return (a & 0x8000000000000000ULL) || !(a & 0x7FFFFFFFFFFFFFFFULL);
-}
-
-uint64_t helper_cmpfgt (uint64_t a)
-{
-    return !(a & 0x8000000000000000ULL) && (a & 0x7FFFFFFFFFFFFFFFULL);
-}
-
-uint64_t helper_cmpfge (uint64_t a)
-{
-    return !(a & 0x8000000000000000ULL) || !(a & 0x7FFFFFFFFFFFFFFFULL);
-}
-
-
 /* Floating point format conversion */
 uint64_t helper_cvtts (uint64_t a)
 {
diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index f9537c9..be7e7bf 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -294,77 +294,98 @@ static inline void gen_store_mem(DisasContext *ctx,
     tcg_temp_free(addr);
 }
 
-static inline void gen_bcond(DisasContext *ctx, TCGCond cond, int ra,
-                             int32_t disp, int mask)
+static void gen_bcond_pcload(DisasContext *ctx, int32_t disp, int lab_true)
 {
-    int l1, l2;
+    int lab_over = gen_new_label();
+
+    tcg_gen_movi_i64(cpu_pc, ctx->pc);
+    tcg_gen_br(lab_over);
+    gen_set_label(lab_true);
+    tcg_gen_movi_i64(cpu_pc, ctx->pc + (int64_t)(disp << 2));
+    gen_set_label(lab_over);
+}
+
+static void gen_bcond(DisasContext *ctx, TCGCond cond, int ra,
+                      int32_t disp, int mask)
+{
+    int lab_true = gen_new_label();
 
-    l1 = gen_new_label();
-    l2 = gen_new_label();
     if (likely(ra != 31)) {
         if (mask) {
             TCGv tmp = tcg_temp_new();
             tcg_gen_andi_i64(tmp, cpu_ir[ra], 1);
-            tcg_gen_brcondi_i64(cond, tmp, 0, l1);
+            tcg_gen_brcondi_i64(cond, tmp, 0, lab_true);
             tcg_temp_free(tmp);
-        } else
-            tcg_gen_brcondi_i64(cond, cpu_ir[ra], 0, l1);
+        } else {
+            tcg_gen_brcondi_i64(cond, cpu_ir[ra], 0, lab_true);
+        }
     } else {
         /* Very uncommon case - Do not bother to optimize.  */
         TCGv tmp = tcg_const_i64(0);
-        tcg_gen_brcondi_i64(cond, tmp, 0, l1);
+        tcg_gen_brcondi_i64(cond, tmp, 0, lab_true);
         tcg_temp_free(tmp);
     }
-    tcg_gen_movi_i64(cpu_pc, ctx->pc);
-    tcg_gen_br(l2);
-    gen_set_label(l1);
-    tcg_gen_movi_i64(cpu_pc, ctx->pc + (int64_t)(disp << 2));
-    gen_set_label(l2);
+    gen_bcond_pcload(ctx, disp, lab_true);
 }
 
-static inline void gen_fbcond(DisasContext *ctx, int opc, int ra, int32_t disp)
+/* Generate a forward TCG branch to LAB_TRUE if RA cmp 0.0.
+   This is complicated by the fact that -0.0 compares the same as +0.0.  */
+
+static void gen_fbcond_internal(TCGCond cond, TCGv src, int lab_true)
 {
-    int l1, l2;
+    int lab_false = -1;
+    uint64_t mzero = 1ull << 63;
     TCGv tmp;
-    TCGv src;
-
-    l1 = gen_new_label();
-    l2 = gen_new_label();
-    if (ra != 31) {
+    
+    switch (cond) {
+    case TCG_COND_LE:
+    case TCG_COND_GT:
+        /* For <= or >, the -0.0 value directly compares the way we want.  */
+        tcg_gen_brcondi_i64(cond, src, 0, lab_true);
+        break;
+
+    case TCG_COND_EQ:
+    case TCG_COND_NE:
+        /* For == or !=, we can simply mask off the sign bit and compare.  */
+        /* ??? Assume that the temporary is reclaimed at the branch.  */
         tmp = tcg_temp_new();
-        src = cpu_fir[ra];
-    } else  {
-        tmp = tcg_const_i64(0);
-        src = tmp;
-    }
-    switch (opc) {
-    case 0x31: /* FBEQ */
-        gen_helper_cmpfeq(tmp, src);
-        break;
-    case 0x32: /* FBLT */
-        gen_helper_cmpflt(tmp, src);
-        break;
-    case 0x33: /* FBLE */
-        gen_helper_cmpfle(tmp, src);
-        break;
-    case 0x35: /* FBNE */
-        gen_helper_cmpfne(tmp, src);
+        tcg_gen_andi_i64(tmp, src, mzero - 1);
+        tcg_gen_brcondi_i64(cond, tmp, 0, lab_true);
         break;
-    case 0x36: /* FBGE */
-        gen_helper_cmpfge(tmp, src);
+
+    case TCG_COND_GE:
+        /* For >=, emit two branches to the destination.  */
+        tcg_gen_brcondi_i64(cond, src, 0, lab_true);
+        tcg_gen_brcondi_i64(TCG_COND_EQ, src, mzero, lab_true);
         break;
-    case 0x37: /* FBGT */
-        gen_helper_cmpfgt(tmp, src);
+
+    case TCG_COND_LT:
+        /* For <, first filter out -0.0 to what will be the fallthru.  */
+        lab_false = gen_new_label();
+        tcg_gen_brcondi_i64(TCG_COND_EQ, src, mzero, lab_false);
+        tcg_gen_brcondi_i64(cond, src, 0, lab_true);
+        gen_set_label(lab_false);
         break;
+
     default:
         abort();
     }
-    tcg_gen_brcondi_i64(TCG_COND_NE, tmp, 0, l1);
-    tcg_gen_movi_i64(cpu_pc, ctx->pc);
-    tcg_gen_br(l2);
-    gen_set_label(l1);
-    tcg_gen_movi_i64(cpu_pc, ctx->pc + (int64_t)(disp << 2));
-    gen_set_label(l2);
+}
+
+static void gen_fbcond(DisasContext *ctx, TCGCond cond, int ra, int32_t disp)
+{
+    int lab_true;
+
+    if (unlikely(ra == 31)) {
+        /* Very uncommon case, but easier to optimize it to an integer
+           comparison than continuing with the floating point comparison.  */
+        gen_bcond(ctx, cond, ra, disp, 0);
+        return;
+    }
+
+    lab_true = gen_new_label();
+    gen_fbcond_internal(cond, cpu_fir[ra], lab_true);
+    gen_bcond_pcload(ctx, disp, lab_true);
 }
 
 static inline void gen_cmov(TCGCond inv_cond, int ra, int rb, int rc,
@@ -399,6 +420,28 @@ static inline void gen_cmov(TCGCond inv_cond, int ra, int rb, int rc,
     gen_set_label(l1);
 }
 
+static void gen_fcmov(TCGCond inv_cond, int ra, int rb, int rc)
+{
+    TCGv va = cpu_fir[ra];
+    int l1;
+
+    if (unlikely(rc == 31))
+        return;
+    if (unlikely(ra == 31)) {
+        /* ??? Assume that the temporary is reclaimed at the branch.  */
+        va = tcg_const_i64(0);
+    }
+
+    l1 = gen_new_label();
+    gen_fbcond_internal(inv_cond, va, l1);
+
+    if (rb != 31)
+        tcg_gen_mov_i64(cpu_fir[rc], cpu_fir[rb]);
+    else
+        tcg_gen_movi_i64(cpu_fir[rc], 0);
+    gen_set_label(l1);
+}
+
 #define FARITH2(name)                                       \
 static inline void glue(gen_f, name)(int rb, int rc)        \
 {                                                           \
@@ -482,38 +525,6 @@ FARITH3(cpys)
 FARITH3(cpysn)
 FARITH3(cpyse)
 
-#define FCMOV(name)                                                   \
-static inline void glue(gen_f, name)(int ra, int rb, int rc)          \
-{                                                                     \
-    int l1;                                                           \
-    TCGv tmp;                                                         \
-                                                                      \
-    if (unlikely(rc == 31))                                           \
-        return;                                                       \
-                                                                      \
-    l1 = gen_new_label();                                             \
-    tmp = tcg_temp_new();                                 \
-    if (ra != 31) {                                                   \
-        tmp = tcg_temp_new();                             \
-        gen_helper_ ## name (tmp, cpu_fir[ra]);                       \
-    } else  {                                                         \
-        tmp = tcg_const_i64(0);                                       \
-        gen_helper_ ## name (tmp, tmp);                               \
-    }                                                                 \
-    tcg_gen_brcondi_i64(TCG_COND_EQ, tmp, 0, l1);                     \
-    if (rb != 31)                                                     \
-        tcg_gen_mov_i64(cpu_fir[rc], cpu_fir[ra]);                    \
-    else                                                              \
-        tcg_gen_movi_i64(cpu_fir[rc], 0);                             \
-    gen_set_label(l1);                                                \
-}
-FCMOV(cmpfeq)
-FCMOV(cmpfne)
-FCMOV(cmpflt)
-FCMOV(cmpfge)
-FCMOV(cmpfle)
-FCMOV(cmpfgt)
-
 static inline uint64_t zapnot_mask(uint8_t lit)
 {
     uint64_t mask = 0;
@@ -1871,27 +1882,27 @@ static inline int translate_one(DisasContext *ctx, uint32_t insn)
             break;
         case 0x02A:
             /* FCMOVEQ */
-            gen_fcmpfeq(ra, rb, rc);
+            gen_fcmov(TCG_COND_NE, ra, rb, rc);
             break;
         case 0x02B:
             /* FCMOVNE */
-            gen_fcmpfne(ra, rb, rc);
+            gen_fcmov(TCG_COND_EQ, ra, rb, rc);
             break;
         case 0x02C:
             /* FCMOVLT */
-            gen_fcmpflt(ra, rb, rc);
+            gen_fcmov(TCG_COND_GE, ra, rb, rc);
             break;
         case 0x02D:
             /* FCMOVGE */
-            gen_fcmpfge(ra, rb, rc);
+            gen_fcmov(TCG_COND_LT, ra, rb, rc);
             break;
         case 0x02E:
             /* FCMOVLE */
-            gen_fcmpfle(ra, rb, rc);
+            gen_fcmov(TCG_COND_GT, ra, rb, rc);
             break;
         case 0x02F:
             /* FCMOVGT */
-            gen_fcmpfgt(ra, rb, rc);
+            gen_fcmov(TCG_COND_LE, ra, rb, rc);
             break;
         case 0x030:
             /* CVTQL */
@@ -2482,9 +2493,15 @@ static inline int translate_one(DisasContext *ctx, uint32_t insn)
         ret = 1;
         break;
     case 0x31: /* FBEQ */
+        gen_fbcond(ctx, TCG_COND_EQ, ra, disp21);
+        ret = 1;
+        break;
     case 0x32: /* FBLT */
+        gen_fbcond(ctx, TCG_COND_LT, ra, disp21);
+        ret = 1;
+        break;
     case 0x33: /* FBLE */
-        gen_fbcond(ctx, opc, ra, disp21);
+        gen_fbcond(ctx, TCG_COND_LE, ra, disp21);
         ret = 1;
         break;
     case 0x34:
@@ -2495,9 +2512,15 @@ static inline int translate_one(DisasContext *ctx, uint32_t insn)
         ret = 1;
         break;
     case 0x35: /* FBNE */
+        gen_fbcond(ctx, TCG_COND_NE, ra, disp21);
+        ret = 1;
+        break;
     case 0x36: /* FBGE */
+        gen_fbcond(ctx, TCG_COND_GE, ra, disp21);
+        ret = 1;
+        break;
     case 0x37: /* FBGT */
-        gen_fbcond(ctx, opc, ra, disp21);
+        gen_fbcond(ctx, TCG_COND_GT, ra, disp21);
         ret = 1;
         break;
     case 0x38:
-- 
1.6.5.2

[Qemu-devel] [PATCH 3/5] target-alpha: Fix generic ctz64.

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 host-utils.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/host-utils.h b/host-utils.h
index e335c5c..0ddc176 100644
--- a/host-utils.h
+++ b/host-utils.h
@@ -164,7 +164,7 @@ static inline int ctz64(uint64_t val)
 {
 #if QEMU_GNUC_PREREQ(3, 4)
     if (val)
-        return __builtin_ctz(val);
+        return __builtin_ctzll(val);
     else
         return 64;
 #else
-- 
1.6.5.2

[Qemu-devel] [PATCH 4/5] target-alpha: Fix cvtlq.

[Richard Henderson]

We were missing the 0xc0000000 mask, leading to incorrect results.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-alpha/op_helper.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/target-alpha/op_helper.c b/target-alpha/op_helper.c
index 8eba5ec..ff120ad 100644
--- a/target-alpha/op_helper.c
+++ b/target-alpha/op_helper.c
@@ -954,7 +954,9 @@ uint64_t helper_cvtqg (uint64_t a)
 
 uint64_t helper_cvtlq (uint64_t a)
 {
-    return (int64_t)((int32_t)((a >> 32) | ((a >> 29) & 0x3FFFFFFF)));
+    int32_t lo = a >> 29;
+    int32_t hi = a >> 32;
+    return (lo & 0x3FFFFFFF) | (hi & 0xc0000000);
 }
 
 static inline uint64_t __helper_cvtql(uint64_t a, int s, int v)
-- 
1.6.5.2

[Qemu-devel] [PATCH 5/5] target-alpha: Fix float32_to_s vs zero exponent.

[Richard Henderson]

There was a bug in float32_to_s that incorrectly mapped a zero exponent
to 0x38.  This meant 0.0f != 0.  At the same time, fix a generic type
punning bug in helper_memory_to_s and helper_s_to_memory.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-alpha/op_helper.c |   44 ++++++++++++++++++++++++++++++++------------
 1 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/target-alpha/op_helper.c b/target-alpha/op_helper.c
index ff120ad..b2abf6c 100644
--- a/target-alpha/op_helper.c
+++ b/target-alpha/op_helper.c
@@ -625,37 +625,57 @@ uint64_t helper_sqrtg (uint64_t a)
 
 
 /* S floating (single) */
+
+/* Taken from linux/arch/alpha/kernel/traps.c, s_mem_to_reg.  */
+static inline uint64_t float32_to_s_int(uint32_t fi)
+{
+    uint32_t frac = fi & 0x7fffff;
+    uint32_t sign = fi >> 31;
+    uint32_t exp_msb = (fi >> 30) & 1;
+    uint32_t exp_low = (fi >> 23) & 0x7f;
+    uint32_t exp;
+
+    exp = (exp_msb << 10) | exp_low;
+    if (exp_msb) {
+        if (exp_low == 0x7f)
+            exp = 0x7ff;
+    } else {
+        if (exp_low != 0x00)
+            exp |= 0x380;
+    }
+
+    return (((uint64_t)sign << 63)
+            | ((uint64_t)exp << 52)
+            | ((uint64_t)frac << 29));
+}
+
 static inline uint64_t float32_to_s(float32 fa)
 {
     CPU_FloatU a;
-    uint64_t r;
-
     a.f = fa;
+    return float32_to_s_int(a.l);
+}
 
-    r = (((uint64_t)(a.l & 0xc0000000)) << 32) | (((uint64_t)(a.l & 0x3fffffff)) << 29);
-    if (((a.l & 0x7f800000) != 0x7f800000) && (!(a.l & 0x40000000)))
-        r |= 0x7ll << 59;
-    return r;
+static inline uint32_t s_to_float32_int(uint64_t a)
+{
+    return ((a >> 32) & 0xc0000000) | ((a >> 29) & 0x3fffffff);
 }
 
 static inline float32 s_to_float32(uint64_t a)
 {
     CPU_FloatU r;
-    r.l = ((a >> 32) & 0xc0000000) | ((a >> 29) & 0x3fffffff);
+    r.l = s_to_float32_int(a);
     return r.f;
 }
 
 uint32_t helper_s_to_memory (uint64_t a)
 {
-    /* Memory format is the same as float32 */
-    float32 fa = s_to_float32(a);
-    return *(uint32_t*)(&fa);
+    return s_to_float32_int(a);
 }
 
 uint64_t helper_memory_to_s (uint32_t a)
 {
-    /* Memory format is the same as float32 */
-    return float32_to_s(*(float32*)(&a));
+    return float32_to_s_int(a);
 }
 
 uint64_t helper_adds (uint64_t a, uint64_t b)
-- 
1.6.5.2