diff mbox series

[v4,082/163] tcg/ppc: Drop fallback constant loading in tcg_out_cmp

Message ID 20250415192515.232910-83-richard.henderson@linaro.org
State New
Headers show
Series tcg: Convert to TCGOutOp structures | expand

Commit Message

Richard Henderson April 15, 2025, 7:23 p.m. UTC 
Use U and C constraints for brcond2 and setcond2, so that
tcg_out_cmp2 automatically passes in-range constants
to tcg_out_cmp.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/ppc/tcg-target-con-set.h |  4 +--
 tcg/ppc/tcg-target.c.inc     | 49 ++++++++++++------------------------
 2 files changed, 18 insertions(+), 35 deletions(-)

Comments

Pierrick Bouvier April 15, 2025, 9:26 p.m. UTC | #1 
On 4/15/25 12:23, Richard Henderson wrote:
> Use U and C constraints for brcond2 and setcond2, so that
> tcg_out_cmp2 automatically passes in-range constants
> to tcg_out_cmp.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tcg/ppc/tcg-target-con-set.h |  4 +--
>   tcg/ppc/tcg-target.c.inc     | 49 ++++++++++++------------------------
>   2 files changed, 18 insertions(+), 35 deletions(-)
> 
> diff --git a/tcg/ppc/tcg-target-con-set.h b/tcg/ppc/tcg-target-con-set.h
> index 77a1038d51..14cd217287 100644
> --- a/tcg/ppc/tcg-target-con-set.h
> +++ b/tcg/ppc/tcg-target-con-set.h
> @@ -15,7 +15,7 @@ C_O0_I2(r, rC)
>   C_O0_I2(v, r)
>   C_O0_I3(r, r, r)
>   C_O0_I3(o, m, r)
> -C_O0_I4(r, r, ri, ri)
> +C_O0_I4(r, r, rU, rC)
>   C_O0_I4(r, r, r, r)
>   C_O1_I1(r, r)
>   C_O1_I1(v, r)
> @@ -34,7 +34,7 @@ C_O1_I2(v, v, v)
>   C_O1_I3(v, v, v, v)
>   C_O1_I4(v, v, v, vZM, v)
>   C_O1_I4(r, r, rC, rZ, rZ)
> -C_O1_I4(r, r, r, ri, ri)
> +C_O1_I4(r, r, r, rU, rC)
>   C_O2_I1(r, r, r)
>   C_N1O1_I1(o, m, r)
>   C_O2_I2(r, r, r, r)
> diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
> index 339b3a0904..1782d05290 100644
> --- a/tcg/ppc/tcg-target.c.inc
> +++ b/tcg/ppc/tcg-target.c.inc
> @@ -1777,9 +1777,8 @@ static void tcg_out_test(TCGContext *s, TCGReg dest, TCGReg arg1, TCGArg arg2,
>   }
>   
>   static void tcg_out_cmp(TCGContext *s, int cond, TCGArg arg1, TCGArg arg2,
> -                        int const_arg2, int cr, TCGType type)
> +                        bool const_arg2, int cr, TCGType type)
>   {
> -    int imm;
>       uint32_t op;
>   
>       tcg_debug_assert(TCG_TARGET_REG_BITS == 64 || type == TCG_TYPE_I32);
> @@ -1796,18 +1795,15 @@ static void tcg_out_cmp(TCGContext *s, int cond, TCGArg arg1, TCGArg arg2,
>       case TCG_COND_EQ:
>       case TCG_COND_NE:
>           if (const_arg2) {
> -            if ((int16_t) arg2 == arg2) {
> +            if ((int16_t)arg2 == arg2) {
>                   op = CMPI;
> -                imm = 1;
> -                break;
> -            } else if ((uint16_t) arg2 == arg2) {
> -                op = CMPLI;
> -                imm = 1;
>                   break;
>               }
> +            tcg_debug_assert((uint16_t)arg2 == arg2);
> +            op = CMPLI;
> +            break;
>           }
>           op = CMPL;
> -        imm = 0;
>           break;
>   
>       case TCG_COND_TSTEQ:
> @@ -1821,14 +1817,11 @@ static void tcg_out_cmp(TCGContext *s, int cond, TCGArg arg1, TCGArg arg2,
>       case TCG_COND_LE:
>       case TCG_COND_GT:
>           if (const_arg2) {
> -            if ((int16_t) arg2 == arg2) {
> -                op = CMPI;
> -                imm = 1;
> -                break;
> -            }
> +            tcg_debug_assert((int16_t)arg2 == arg2);
> +            op = CMPI;
> +            break;
>           }
>           op = CMP;
> -        imm = 0;
>           break;
>   
>       case TCG_COND_LTU:
> @@ -1836,30 +1829,20 @@ static void tcg_out_cmp(TCGContext *s, int cond, TCGArg arg1, TCGArg arg2,
>       case TCG_COND_LEU:
>       case TCG_COND_GTU:
>           if (const_arg2) {
> -            if ((uint16_t) arg2 == arg2) {
> -                op = CMPLI;
> -                imm = 1;
> -                break;
> -            }
> +            tcg_debug_assert((uint16_t)arg2 == arg2);
> +            op = CMPLI;
> +            break;
>           }
>           op = CMPL;
> -        imm = 0;
>           break;
>   
>       default:
>           g_assert_not_reached();
>       }
>       op |= BF(cr) | ((type == TCG_TYPE_I64) << 21);
> -
> -    if (imm) {
> -        tcg_out32(s, op | RA(arg1) | (arg2 & 0xffff));
> -    } else {
> -        if (const_arg2) {
> -            tcg_out_movi(s, type, TCG_REG_R0, arg2);
> -            arg2 = TCG_REG_R0;
> -        }
> -        tcg_out32(s, op | RA(arg1) | RB(arg2));
> -    }
> +    op |= RA(arg1);
> +    op |= const_arg2 ? arg2 & 0xffff : RB(arg2);
> +    tcg_out32(s, op);
>   }
>   
>   static void tcg_out_setcond_eq0(TCGContext *s, TCGType type,
> @@ -4297,9 +4280,9 @@ tcg_target_op_def(TCGOpcode op, TCGType type, unsigned flags)
>       case INDEX_op_deposit_i64:
>           return C_O1_I2(r, 0, rZ);
>       case INDEX_op_brcond2_i32:
> -        return C_O0_I4(r, r, ri, ri);
> +        return C_O0_I4(r, r, rU, rC);
>       case INDEX_op_setcond2_i32:
> -        return C_O1_I4(r, r, r, ri, ri);
> +        return C_O1_I4(r, r, r, rU, rC);
>       case INDEX_op_add2_i64:
>       case INDEX_op_add2_i32:
>           return C_O2_I4(r, r, r, r, rI, rZM);

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Nicholas Piggin April 16, 2025, 2:39 p.m. UTC | #2 
On Wed Apr 16, 2025 at 5:23 AM AEST, Richard Henderson wrote:
> Use U and C constraints for brcond2 and setcond2, so that
> tcg_out_cmp2 automatically passes in-range constants
> to tcg_out_cmp.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/ppc/tcg-target-con-set.h |  4 +--
>  tcg/ppc/tcg-target.c.inc     | 49 ++++++++++++------------------------
>  2 files changed, 18 insertions(+), 35 deletions(-)
>
> diff --git a/tcg/ppc/tcg-target-con-set.h b/tcg/ppc/tcg-target-con-set.h
> index 77a1038d51..14cd217287 100644
> --- a/tcg/ppc/tcg-target-con-set.h
> +++ b/tcg/ppc/tcg-target-con-set.h
> @@ -15,7 +15,7 @@ C_O0_I2(r, rC)
>  C_O0_I2(v, r)
>  C_O0_I3(r, r, r)
>  C_O0_I3(o, m, r)
> -C_O0_I4(r, r, ri, ri)
> +C_O0_I4(r, r, rU, rC)
>  C_O0_I4(r, r, r, r)
>  C_O1_I1(r, r)
>  C_O1_I1(v, r)
> @@ -34,7 +34,7 @@ C_O1_I2(v, v, v)
>  C_O1_I3(v, v, v, v)
>  C_O1_I4(v, v, v, vZM, v)
>  C_O1_I4(r, r, rC, rZ, rZ)
> -C_O1_I4(r, r, r, ri, ri)
> +C_O1_I4(r, r, r, rU, rC)
>  C_O2_I1(r, r, r)
>  C_N1O1_I1(o, m, r)
>  C_O2_I2(r, r, r, r)
> diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
> index 339b3a0904..1782d05290 100644
> --- a/tcg/ppc/tcg-target.c.inc
> +++ b/tcg/ppc/tcg-target.c.inc
> @@ -1777,9 +1777,8 @@ static void tcg_out_test(TCGContext *s, TCGReg dest, TCGReg arg1, TCGArg arg2,
>  }
>  
>  static void tcg_out_cmp(TCGContext *s, int cond, TCGArg arg1, TCGArg arg2,
> -                        int const_arg2, int cr, TCGType type)
> +                        bool const_arg2, int cr, TCGType type)
>  {
> -    int imm;
>      uint32_t op;
>  
>      tcg_debug_assert(TCG_TARGET_REG_BITS == 64 || type == TCG_TYPE_I32);
> @@ -1796,18 +1795,15 @@ static void tcg_out_cmp(TCGContext *s, int cond, TCGArg arg1, TCGArg arg2,
>      case TCG_COND_EQ:
>      case TCG_COND_NE:
>          if (const_arg2) {
> -            if ((int16_t) arg2 == arg2) {
> +            if ((int16_t)arg2 == arg2) {
>                  op = CMPI;
> -                imm = 1;
> -                break;
> -            } else if ((uint16_t) arg2 == arg2) {
> -                op = CMPLI;
> -                imm = 1;
>                  break;
>              }
> +            tcg_debug_assert((uint16_t)arg2 == arg2);
> +            op = CMPLI;
> +            break;
>          }
>          op = CMPL;
> -        imm = 0;
>          break;
>  
>      case TCG_COND_TSTEQ:
> @@ -1821,14 +1817,11 @@ static void tcg_out_cmp(TCGContext *s, int cond, TCGArg arg1, TCGArg arg2,
>      case TCG_COND_LE:
>      case TCG_COND_GT:
>          if (const_arg2) {
> -            if ((int16_t) arg2 == arg2) {
> -                op = CMPI;
> -                imm = 1;
> -                break;
> -            }
> +            tcg_debug_assert((int16_t)arg2 == arg2);
> +            op = CMPI;
> +            break;
>          }
>          op = CMP;
> -        imm = 0;
>          break;
>  
>      case TCG_COND_LTU:
> @@ -1836,30 +1829,20 @@ static void tcg_out_cmp(TCGContext *s, int cond, TCGArg arg1, TCGArg arg2,
>      case TCG_COND_LEU:
>      case TCG_COND_GTU:
>          if (const_arg2) {
> -            if ((uint16_t) arg2 == arg2) {
> -                op = CMPLI;
> -                imm = 1;
> -                break;
> -            }
> +            tcg_debug_assert((uint16_t)arg2 == arg2);
> +            op = CMPLI;
> +            break;
>          }
>          op = CMPL;
> -        imm = 0;
>          break;
>  
>      default:
>          g_assert_not_reached();
>      }
>      op |= BF(cr) | ((type == TCG_TYPE_I64) << 21);
> -
> -    if (imm) {
> -        tcg_out32(s, op | RA(arg1) | (arg2 & 0xffff));
> -    } else {
> -        if (const_arg2) {
> -            tcg_out_movi(s, type, TCG_REG_R0, arg2);
> -            arg2 = TCG_REG_R0;
> -        }
> -        tcg_out32(s, op | RA(arg1) | RB(arg2));
> -    }
> +    op |= RA(arg1);
> +    op |= const_arg2 ? arg2 & 0xffff : RB(arg2);

Looks good as far as I can see (I don't know the backend very well).

arg2 should not ever have upper bits set here (nor in the code you
replaced), right?

FWIW,

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> +    tcg_out32(s, op);
>  }
>  
>  static void tcg_out_setcond_eq0(TCGContext *s, TCGType type,
> @@ -4297,9 +4280,9 @@ tcg_target_op_def(TCGOpcode op, TCGType type, unsigned flags)
>      case INDEX_op_deposit_i64:
>          return C_O1_I2(r, 0, rZ);
>      case INDEX_op_brcond2_i32:
> -        return C_O0_I4(r, r, ri, ri);
> +        return C_O0_I4(r, r, rU, rC);
>      case INDEX_op_setcond2_i32:
> -        return C_O1_I4(r, r, r, ri, ri);
> +        return C_O1_I4(r, r, r, rU, rC);
>      case INDEX_op_add2_i64:
>      case INDEX_op_add2_i32:
>          return C_O2_I4(r, r, r, r, rI, rZM);
Richard Henderson April 16, 2025, 6:57 p.m. UTC | #3 
On 4/16/25 07:39, Nicholas Piggin wrote:
>> +    op |= RA(arg1);
>> +    op |= const_arg2 ? arg2 & 0xffff : RB(arg2);
> 
> Looks good as far as I can see (I don't know the backend very well).
> 
> arg2 should not ever have upper bits set here (nor in the code you
> replaced), right?

Only in the sense of int16_t sign-extended to TCGArg.
This will have been checked by constraints.


r~
diff mbox series

Patch

diff --git a/tcg/ppc/tcg-target-con-set.h b/tcg/ppc/tcg-target-con-set.h
index 77a1038d51..14cd217287 100644
--- a/tcg/ppc/tcg-target-con-set.h
+++ b/tcg/ppc/tcg-target-con-set.h
@@ -15,7 +15,7 @@  C_O0_I2(r, rC)
 C_O0_I2(v, r)
 C_O0_I3(r, r, r)
 C_O0_I3(o, m, r)
-C_O0_I4(r, r, ri, ri)
+C_O0_I4(r, r, rU, rC)
 C_O0_I4(r, r, r, r)
 C_O1_I1(r, r)
 C_O1_I1(v, r)
@@ -34,7 +34,7 @@  C_O1_I2(v, v, v)
 C_O1_I3(v, v, v, v)
 C_O1_I4(v, v, v, vZM, v)
 C_O1_I4(r, r, rC, rZ, rZ)
-C_O1_I4(r, r, r, ri, ri)
+C_O1_I4(r, r, r, rU, rC)
 C_O2_I1(r, r, r)
 C_N1O1_I1(o, m, r)
 C_O2_I2(r, r, r, r)
diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
index 339b3a0904..1782d05290 100644
--- a/tcg/ppc/tcg-target.c.inc
+++ b/tcg/ppc/tcg-target.c.inc
@@ -1777,9 +1777,8 @@  static void tcg_out_test(TCGContext *s, TCGReg dest, TCGReg arg1, TCGArg arg2,
 }
 
 static void tcg_out_cmp(TCGContext *s, int cond, TCGArg arg1, TCGArg arg2,
-                        int const_arg2, int cr, TCGType type)
+                        bool const_arg2, int cr, TCGType type)
 {
-    int imm;
     uint32_t op;
 
     tcg_debug_assert(TCG_TARGET_REG_BITS == 64 || type == TCG_TYPE_I32);
@@ -1796,18 +1795,15 @@  static void tcg_out_cmp(TCGContext *s, int cond, TCGArg arg1, TCGArg arg2,
     case TCG_COND_EQ:
     case TCG_COND_NE:
         if (const_arg2) {
-            if ((int16_t) arg2 == arg2) {
+            if ((int16_t)arg2 == arg2) {
                 op = CMPI;
-                imm = 1;
-                break;
-            } else if ((uint16_t) arg2 == arg2) {
-                op = CMPLI;
-                imm = 1;
                 break;
             }
+            tcg_debug_assert((uint16_t)arg2 == arg2);
+            op = CMPLI;
+            break;
         }
         op = CMPL;
-        imm = 0;
         break;
 
     case TCG_COND_TSTEQ:
@@ -1821,14 +1817,11 @@  static void tcg_out_cmp(TCGContext *s, int cond, TCGArg arg1, TCGArg arg2,
     case TCG_COND_LE:
     case TCG_COND_GT:
         if (const_arg2) {
-            if ((int16_t) arg2 == arg2) {
-                op = CMPI;
-                imm = 1;
-                break;
-            }
+            tcg_debug_assert((int16_t)arg2 == arg2);
+            op = CMPI;
+            break;
         }
         op = CMP;
-        imm = 0;
         break;
 
     case TCG_COND_LTU:
@@ -1836,30 +1829,20 @@  static void tcg_out_cmp(TCGContext *s, int cond, TCGArg arg1, TCGArg arg2,
     case TCG_COND_LEU:
     case TCG_COND_GTU:
         if (const_arg2) {
-            if ((uint16_t) arg2 == arg2) {
-                op = CMPLI;
-                imm = 1;
-                break;
-            }
+            tcg_debug_assert((uint16_t)arg2 == arg2);
+            op = CMPLI;
+            break;
         }
         op = CMPL;
-        imm = 0;
         break;
 
     default:
         g_assert_not_reached();
     }
     op |= BF(cr) | ((type == TCG_TYPE_I64) << 21);
-
-    if (imm) {
-        tcg_out32(s, op | RA(arg1) | (arg2 & 0xffff));
-    } else {
-        if (const_arg2) {
-            tcg_out_movi(s, type, TCG_REG_R0, arg2);
-            arg2 = TCG_REG_R0;
-        }
-        tcg_out32(s, op | RA(arg1) | RB(arg2));
-    }
+    op |= RA(arg1);
+    op |= const_arg2 ? arg2 & 0xffff : RB(arg2);
+    tcg_out32(s, op);
 }
 
 static void tcg_out_setcond_eq0(TCGContext *s, TCGType type,
@@ -4297,9 +4280,9 @@  tcg_target_op_def(TCGOpcode op, TCGType type, unsigned flags)
     case INDEX_op_deposit_i64:
         return C_O1_I2(r, 0, rZ);
     case INDEX_op_brcond2_i32:
-        return C_O0_I4(r, r, ri, ri);
+        return C_O0_I4(r, r, rU, rC);
     case INDEX_op_setcond2_i32:
-        return C_O1_I4(r, r, r, ri, ri);
+        return C_O1_I4(r, r, r, rU, rC);
     case INDEX_op_add2_i64:
     case INDEX_op_add2_i32:
         return C_O2_I4(r, r, r, r, rI, rZM);