Patchwork [1/2] tcg-hppa: Fix brcond2 and setcond2

login
register
mail settings
Submitter Richard Henderson
Date Sept. 19, 2012, 2:59 a.m.
Message ID <1348023588-6764-2-git-send-email-rth@twiddle.net>
Download mbox | patch
Permalink /patch/184920/
State New
Headers show

Comments

Richard Henderson - Sept. 19, 2012, 2:59 a.m.
Neither of these functions were performing double-word
compares properly.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/hppa/tcg-target.c | 51 ++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 9 deletions(-)
Stuart Brady - Dec. 24, 2012, 10:26 p.m.
On Tue, Sep 18, 2012 at 07:59:47PM -0700, Richard Henderson wrote:
> diff --git a/tcg/hppa/tcg-target.c b/tcg/hppa/tcg-target.c
> index 8b81b70..a76569d 100644
> --- a/tcg/hppa/tcg-target.c
> +++ b/tcg/hppa/tcg-target.c
> @@ -820,19 +820,34 @@ static void tcg_out_comclr(TCGContext *s, int cond, TCGArg ret,
>      tcg_out32(s, op);
>  }
[...]
>      case TCG_COND_NE:
> -        tcg_out_comclr(s, tcg_invert_cond(cond), TCG_REG_R0, al, bl, blconst);
> -        tcg_out_brcond(s, cond, ah, bh, bhconst, label_index);
> +        tcg_out_brcond(s, TCG_COND_NE, al, bl, bhconst, label_index);
> +        tcg_out_brcond(s, TCG_COND_NE, ah, bh, bhconst, label_index);
>          break;

I'm not sure whether it's possible to have blconst != bhconst, but I see
that 'bhconst' is accidentally used together with 'al' and 'bl' here.
It may be some time before I get around to submitting a patch for this,
so just mentioning it briefly.

Patch

diff --git a/tcg/hppa/tcg-target.c b/tcg/hppa/tcg-target.c
index 8b81b70..a76569d 100644
--- a/tcg/hppa/tcg-target.c
+++ b/tcg/hppa/tcg-target.c
@@ -820,19 +820,34 @@  static void tcg_out_comclr(TCGContext *s, int cond, TCGArg ret,
     tcg_out32(s, op);
 }
 
+static TCGCond const tcg_high_cond[] = {
+    [TCG_COND_EQ] = TCG_COND_EQ,
+    [TCG_COND_NE] = TCG_COND_NE,
+    [TCG_COND_LT] = TCG_COND_LT,
+    [TCG_COND_LE] = TCG_COND_LT,
+    [TCG_COND_GT] = TCG_COND_GT,
+    [TCG_COND_GE] = TCG_COND_GT,
+    [TCG_COND_LTU] = TCG_COND_LTU,
+    [TCG_COND_LEU] = TCG_COND_LTU,
+    [TCG_COND_GTU] = TCG_COND_GTU,
+    [TCG_COND_GEU] = TCG_COND_GTU
+};
+
 static void tcg_out_brcond2(TCGContext *s, int cond, TCGArg al, TCGArg ah,
                             TCGArg bl, int blconst, TCGArg bh, int bhconst,
                             int label_index)
 {
     switch (cond) {
     case TCG_COND_EQ:
+        tcg_out_comclr(s, TCG_COND_NE, TCG_REG_R0, al, bl, blconst);
+        tcg_out_brcond(s, TCG_COND_EQ, ah, bh, bhconst, label_index);
+        break;
     case TCG_COND_NE:
-        tcg_out_comclr(s, tcg_invert_cond(cond), TCG_REG_R0, al, bl, blconst);
-        tcg_out_brcond(s, cond, ah, bh, bhconst, label_index);
+        tcg_out_brcond(s, TCG_COND_NE, al, bl, bhconst, label_index);
+        tcg_out_brcond(s, TCG_COND_NE, ah, bh, bhconst, label_index);
         break;
-
     default:
-        tcg_out_brcond(s, cond, ah, bh, bhconst, label_index);
+        tcg_out_brcond(s, tcg_high_cond[cond], ah, bh, bhconst, label_index);
         tcg_out_comclr(s, TCG_COND_NE, TCG_REG_R0, ah, bh, bhconst);
         tcg_out_brcond(s, tcg_unsigned_cond(cond),
                        al, bl, blconst, label_index);
@@ -853,9 +868,8 @@  static void tcg_out_setcond2(TCGContext *s, int cond, TCGArg ret,
 {
     int scratch = TCG_REG_R20;
 
-    if (ret != al && ret != ah
-        && (blconst || ret != bl)
-        && (bhconst || ret != bh)) {
+    /* Note that the low parts are fully consumed before scratch is set.  */
+    if (ret != ah && (bhconst || ret != bh)) {
         scratch = ret;
     }
 
@@ -867,13 +881,32 @@  static void tcg_out_setcond2(TCGContext *s, int cond, TCGArg ret,
         tcg_out_movi(s, TCG_TYPE_I32, scratch, cond == TCG_COND_NE);
         break;
 
-    default:
+    case TCG_COND_GE:
+    case TCG_COND_GEU:
+    case TCG_COND_LT:
+    case TCG_COND_LTU:
+        /* Optimize compares with low part zero.  */
+        if (bl == 0) {
+            tcg_out_setcond(s, cond, ret, ah, bh, bhconst);
+            return;
+        }
+        /* FALLTHRU */
+
+    case TCG_COND_LE:
+    case TCG_COND_LEU:
+    case TCG_COND_GT:
+    case TCG_COND_GTU:
+        /* <= : ah < bh | (ah == bh && al <= bl) */
         tcg_out_setcond(s, tcg_unsigned_cond(cond), scratch, al, bl, blconst);
         tcg_out_comclr(s, TCG_COND_EQ, TCG_REG_R0, ah, bh, bhconst);
         tcg_out_movi(s, TCG_TYPE_I32, scratch, 0);
-        tcg_out_comclr(s, cond, TCG_REG_R0, ah, bh, bhconst);
+        tcg_out_comclr(s, tcg_invert_cond(tcg_high_cond[cond]),
+                       TCG_REG_R0, ah, bh, bhconst);
         tcg_out_movi(s, TCG_TYPE_I32, scratch, 1);
         break;
+
+    default:
+        tcg_abort();
     }
 
     tcg_out_mov(s, TCG_TYPE_I32, ret, scratch);