[PATCH 00/12] tcg: Factor hrev{32,64}_{i32,i64,tl} out

[PATCH 00/12] tcg: Factor hrev{32,64}_{i32,i64,tl} out

[Philippe Mathieu-Daudé]

This series factor the "byteswap each halfword within a
32/64-bit value" code duplication as generic helpers.

Modulo the documentation added, there is a good negative
diff-stat, so I believe this is a win from a maintainance
point of view.

I used "hrev" to follow the other bswap/hswap/rev helpers
but it isn't very descriptive, so any better name suggestion
is welcomed.
(In particular because there are other patterns I'd like to
factor out and then naming is getting worse, such 'wrev').

Philippe Mathieu-Daudé (12):
  tcg/tcg-op: Factor tcg_gen_hrev32_i32() out
  target/arm: Use generic hrev32_i32() in ARM REV16 and VREV16 opcodes
  target/cris: Use generic hrev32_i32() in SWAPB opcode
  target/rx: Use generic hrev32_i32() in REVW opcode
  tcg/tcg-op: Factor tcg_gen_hrev64_i64() out
  target/mips: Use generic hrev64_i64() in DSBH opcode
  target/ppc: Use generic hrev64_i64() in BRH / BSWAP16x8 opcodes
  target/loongarch: Use generic hrev64_i64() in REVB.4H opcode
  tcg/tcg-op: Add tcg_gen_hrev32_i64() and tcg_gen_hrev_i64()
  target/arm: Use generic hrev_i64() in Aarch64 REV16 opcode
  target/loongarch: Use generic hrev64_i32() in REVB.2H opcode
  target/mips: Use generic hrev32_tl() in WSBH opcode

 docs/devel/tcg-ops.rst                      | 10 +++
 include/tcg/tcg-op-common.h                 |  4 +
 include/tcg/tcg-op.h                        |  2 +
 target/arm/tcg/translate-a32.h              |  1 -
 target/arm/tcg/translate-a64.c              | 11 +--
 target/arm/tcg/translate-neon.c             |  2 +-
 target/arm/tcg/translate.c                  | 14 +---
 target/cris/translate.c                     | 20 +----
 target/mips/tcg/translate.c                 | 24 +-----
 target/ppc/translate.c                      | 10 +--
 target/rx/translate.c                       |  8 +-
 tcg/tcg-op.c                                | 81 ++++++++++++++++++---
 target/cris/translate_v10.c.inc             |  2 +-
 target/loongarch/insn_trans/trans_bit.c.inc | 30 +-------
 target/ppc/translate/vsx-impl.c.inc         | 19 +----
 15 files changed, 99 insertions(+), 139 deletions(-)

-- 
2.41.0

[PATCH 01/12] tcg/tcg-op: Factor tcg_gen_hrev32_i32() out

[Philippe Mathieu-Daudé]

Byteswapping each halfword is a common operation, so
extract to a new tcg_gen_hrev32_i32() generic helper.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 docs/devel/tcg-ops.rst      |  4 ++++
 include/tcg/tcg-op-common.h |  1 +
 tcg/tcg-op.c                | 29 +++++++++++++++++++++++------
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/docs/devel/tcg-ops.rst b/docs/devel/tcg-ops.rst
index 7ea6aba502..17965faa03 100644
--- a/docs/devel/tcg-ops.rst
+++ b/docs/devel/tcg-ops.rst
@@ -490,6 +490,10 @@ Misc
          into 32-bit output *t0*.  Depending on the host, this may be a simple shift,
          or may require additional canonicalization.
 
+   * - hrev32_i32 *t0*, *t1*
+
+     - | Byteswap each halfword within a 32-bit value.
+
    * - hswap_i32/i64 *t0*, *t1*
 
      - | Swap 16-bit halfwords within a 32/64-bit value.
diff --git a/include/tcg/tcg-op-common.h b/include/tcg/tcg-op-common.h
index be382bbf77..bb515dfd51 100644
--- a/include/tcg/tcg-op-common.h
+++ b/include/tcg/tcg-op-common.h
@@ -360,6 +360,7 @@ void tcg_gen_ext16u_i32(TCGv_i32 ret, TCGv_i32 arg);
 void tcg_gen_bswap16_i32(TCGv_i32 ret, TCGv_i32 arg, int flags);
 void tcg_gen_bswap32_i32(TCGv_i32 ret, TCGv_i32 arg);
 void tcg_gen_hswap_i32(TCGv_i32 ret, TCGv_i32 arg);
+void tcg_gen_hrev32_i32(TCGv_i32 ret, TCGv_i32 arg);
 void tcg_gen_smin_i32(TCGv_i32, TCGv_i32 arg1, TCGv_i32 arg2);
 void tcg_gen_smax_i32(TCGv_i32, TCGv_i32 arg1, TCGv_i32 arg2);
 void tcg_gen_umin_i32(TCGv_i32, TCGv_i32 arg1, TCGv_i32 arg2);
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index c436c5e263..b1b5d9b45b 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -1073,14 +1073,9 @@ void tcg_gen_bswap32_i32(TCGv_i32 ret, TCGv_i32 arg)
     } else {
         TCGv_i32 t0 = tcg_temp_ebb_new_i32();
         TCGv_i32 t1 = tcg_temp_ebb_new_i32();
-        TCGv_i32 t2 = tcg_constant_i32(0x00ff00ff);
 
                                         /* arg = abcd */
-        tcg_gen_shri_i32(t0, arg, 8);   /*  t0 = .abc */
-        tcg_gen_and_i32(t1, arg, t2);   /*  t1 = .b.d */
-        tcg_gen_and_i32(t0, t0, t2);    /*  t0 = .a.c */
-        tcg_gen_shli_i32(t1, t1, 8);    /*  t1 = b.d. */
-        tcg_gen_or_i32(ret, t0, t1);    /* ret = badc */
+        tcg_gen_hrev32_i32(ret, arg);   /* ret = badc */
 
         tcg_gen_shri_i32(t0, ret, 16);  /*  t0 = ..ba */
         tcg_gen_shli_i32(t1, ret, 16);  /*  t1 = dc.. */
@@ -1102,6 +1097,28 @@ void tcg_gen_hswap_i32(TCGv_i32 ret, TCGv_i32 arg)
     tcg_gen_rotli_i32(ret, arg, 16);
 }
 
+/*
+ * hswap_i32: Byteswap each halfword within a 32-bit value.
+ *
+ * Byte pattern:  hswap_i32(abcd) -> badc
+ */
+void tcg_gen_hrev32_i32(TCGv_i32 ret, TCGv_i32 arg)
+{
+    TCGv_i32 mask = tcg_constant_i32(0x00ff00ff);
+    TCGv_i32 t0 = tcg_temp_ebb_new_i32();
+    TCGv_i32 t1 = tcg_temp_ebb_new_i32();
+
+                                        /* arg = abcd */
+    tcg_gen_shri_i32(t0, arg, 8);       /*  t0 = .abc */
+    tcg_gen_and_i32(t1, arg, mask);     /*  t1 = .b.d */
+    tcg_gen_and_i32(t0, t0, mask);      /*  t0 = .a.c */
+    tcg_gen_shli_i32(t1, t1, 8);        /*  t1 = b.d. */
+    tcg_gen_or_i32(ret, t0, t1);        /* ret = badc */
+
+    tcg_temp_free_i32(t0);
+    tcg_temp_free_i32(t1);
+}
+
 void tcg_gen_smin_i32(TCGv_i32 ret, TCGv_i32 a, TCGv_i32 b)
 {
     tcg_gen_movcond_i32(TCG_COND_LT, ret, a, b, a, b);
-- 
2.41.0

[PATCH 02/12] target/arm: Use generic hrev32_i32() in ARM REV16 and VREV16 opcodes

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/tcg/translate-a32.h  |  1 -
 target/arm/tcg/translate-neon.c |  2 +-
 target/arm/tcg/translate.c      | 14 +-------------
 3 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/target/arm/tcg/translate-a32.h b/target/arm/tcg/translate-a32.h
index 0c8f408eea..6cc02c83b9 100644
--- a/target/arm/tcg/translate-a32.h
+++ b/target/arm/tcg/translate-a32.h
@@ -45,7 +45,6 @@ void gen_lookup_tb(DisasContext *s);
 long vfp_reg_offset(bool dp, unsigned reg);
 long neon_full_reg_offset(unsigned reg);
 long neon_element_offset(int reg, int element, MemOp memop);
-void gen_rev16(TCGv_i32 dest, TCGv_i32 var);
 void clear_eci_state(DisasContext *s);
 bool mve_eci_check(DisasContext *s);
 void mve_update_eci(DisasContext *s);
diff --git a/target/arm/tcg/translate-neon.c b/target/arm/tcg/translate-neon.c
index 0e59b03ff9..9489dc0b3e 100644
--- a/target/arm/tcg/translate-neon.c
+++ b/target/arm/tcg/translate-neon.c
@@ -3528,7 +3528,7 @@ static bool trans_VREV16(DisasContext *s, arg_2misc *a)
     if (a->size != 0) {
         return false;
     }
-    return do_2misc(s, a, gen_rev16);
+    return do_2misc(s, a, tcg_gen_hrev32_i32);
 }
 
 static bool trans_VCLS(DisasContext *s, arg_2misc *a)
diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
index 39a42611c6..4ebf04f4de 100644
--- a/target/arm/tcg/translate.c
+++ b/target/arm/tcg/translate.c
@@ -416,18 +416,6 @@ static void gen_smul_dual(TCGv_i32 a, TCGv_i32 b)
     tcg_gen_mov_i32(a, tmp1);
 }
 
-/* Byteswap each halfword.  */
-void gen_rev16(TCGv_i32 dest, TCGv_i32 var)
-{
-    TCGv_i32 tmp = tcg_temp_new_i32();
-    TCGv_i32 mask = tcg_constant_i32(0x00ff00ff);
-    tcg_gen_shri_i32(tmp, var, 8);
-    tcg_gen_and_i32(tmp, tmp, mask);
-    tcg_gen_and_i32(var, var, mask);
-    tcg_gen_shli_i32(var, var, 8);
-    tcg_gen_or_i32(dest, var, tmp);
-}
-
 /* Byteswap low halfword and sign extend.  */
 static void gen_revsh(TCGv_i32 dest, TCGv_i32 var)
 {
@@ -7578,7 +7566,7 @@ static bool trans_REV16(DisasContext *s, arg_rr *a)
     if (!ENABLE_ARCH_6) {
         return false;
     }
-    return op_rr(s, a, gen_rev16);
+    return op_rr(s, a, tcg_gen_hrev32_i32);
 }
 
 static bool trans_REVSH(DisasContext *s, arg_rr *a)
-- 
2.41.0

[PATCH 03/12] target/cris: Use generic hrev32_i32() in SWAPB opcode

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/cris/translate.c         | 20 +-------------------
 target/cris/translate_v10.c.inc |  2 +-
 2 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/target/cris/translate.c b/target/cris/translate.c
index 925ed2c6f6..00bbe6c645 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -381,24 +381,6 @@ static inline void t_gen_subx_carry(DisasContext *dc, TCGv d)
     }
 }
 
-/* Swap the two bytes within each half word of the s operand.
-   T0 = ((T0 << 8) & 0xff00ff00) | ((T0 >> 8) & 0x00ff00ff)  */
-static inline void t_gen_swapb(TCGv d, TCGv s)
-{
-    TCGv t, org_s;
-
-    t = tcg_temp_new();
-    org_s = tcg_temp_new();
-
-    /* d and s may refer to the same object.  */
-    tcg_gen_mov_tl(org_s, s);
-    tcg_gen_shli_tl(t, org_s, 8);
-    tcg_gen_andi_tl(d, t, 0xff00ff00);
-    tcg_gen_shri_tl(t, org_s, 8);
-    tcg_gen_andi_tl(t, t, 0x00ff00ff);
-    tcg_gen_or_tl(d, d, t);
-}
-
 /*
  * Reverse the bits within each byte.
  *
@@ -1666,7 +1648,7 @@ static int dec_swap_r(CPUCRISState *env, DisasContext *dc)
         tcg_gen_hswap_i32(t0, t0);
     }
     if (dc->op2 & 2) {
-        t_gen_swapb(t0, t0);
+        tcg_gen_hrev32_i32(t0, t0);
     }
     if (dc->op2 & 1) {
         t_gen_swapr(t0, t0);
diff --git a/target/cris/translate_v10.c.inc b/target/cris/translate_v10.c.inc
index 0ff15769ec..86cc5de8ad 100644
--- a/target/cris/translate_v10.c.inc
+++ b/target/cris/translate_v10.c.inc
@@ -508,7 +508,7 @@ static void dec10_reg_swap(DisasContext *dc)
     if (dc->dst & 4)
         tcg_gen_hswap_i32(t0, t0);
     if (dc->dst & 2)
-        t_gen_swapb(t0, t0);
+        tcg_gen_hrev32_i32(t0, t0);
     if (dc->dst & 1)
         t_gen_swapr(t0, t0);
     cris_alu(dc, CC_OP_MOVE, cpu_R[dc->src], cpu_R[dc->src], t0, 4);
-- 
2.41.0

Re: [PATCH 03/12] target/cris: Use generic hrev32_i32() in SWAPB opcode

[Philippe Mathieu-Daudé]

On 22/8/23 14:40, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/cris/translate.c         | 20 +-------------------
>   target/cris/translate_v10.c.inc |  2 +-
>   2 files changed, 2 insertions(+), 20 deletions(-)
> 
> diff --git a/target/cris/translate.c b/target/cris/translate.c
> index 925ed2c6f6..00bbe6c645 100644
> --- a/target/cris/translate.c
> +++ b/target/cris/translate.c
> @@ -381,24 +381,6 @@ static inline void t_gen_subx_carry(DisasContext *dc, TCGv d)
>       }
>   }
>   
> -/* Swap the two bytes within each half word of the s operand.
> -   T0 = ((T0 << 8) & 0xff00ff00) | ((T0 >> 8) & 0x00ff00ff)  */
> -static inline void t_gen_swapb(TCGv d, TCGv s)
> -{
> -    TCGv t, org_s;
> -
> -    t = tcg_temp_new();
> -    org_s = tcg_temp_new();
> -
> -    /* d and s may refer to the same object.  */
> -    tcg_gen_mov_tl(org_s, s);
> -    tcg_gen_shli_tl(t, org_s, 8);
> -    tcg_gen_andi_tl(d, t, 0xff00ff00);
> -    tcg_gen_shri_tl(t, org_s, 8);
> -    tcg_gen_andi_tl(t, t, 0x00ff00ff);
> -    tcg_gen_or_tl(d, d, t);
> -}
> -
>   /*
>    * Reverse the bits within each byte.
>    *
> @@ -1666,7 +1648,7 @@ static int dec_swap_r(CPUCRISState *env, DisasContext *dc)
>           tcg_gen_hswap_i32(t0, t0);
>       }
>       if (dc->op2 & 2) {
> -        t_gen_swapb(t0, t0);
> +        tcg_gen_hrev32_i32(t0, t0);

Here we should use tcg_gen_hrev32_tl (added later in this series).

>       }

[PATCH 04/12] target/rx: Use generic hrev32_i32() in REVW opcode

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/rx/translate.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/target/rx/translate.c b/target/rx/translate.c
index f552a0319a..75590ae05e 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -1513,13 +1513,7 @@ static bool trans_REVL(DisasContext *ctx, arg_REVL *a)
 /* revw rs, rd */
 static bool trans_REVW(DisasContext *ctx, arg_REVW *a)
 {
-    TCGv tmp;
-    tmp = tcg_temp_new();
-    tcg_gen_andi_i32(tmp, cpu_regs[a->rs], 0x00ff00ff);
-    tcg_gen_shli_i32(tmp, tmp, 8);
-    tcg_gen_shri_i32(cpu_regs[a->rd], cpu_regs[a->rs], 8);
-    tcg_gen_andi_i32(cpu_regs[a->rd], cpu_regs[a->rd], 0x00ff00ff);
-    tcg_gen_or_i32(cpu_regs[a->rd], cpu_regs[a->rd], tmp);
+    tcg_gen_hrev32_i32(cpu_regs[a->rd], cpu_regs[a->rs]);
     return true;
 }
 
-- 
2.41.0

[PATCH 05/12] tcg/tcg-op: Factor tcg_gen_hrev64_i64() out

[Philippe Mathieu-Daudé]

Similarly to tcg_gen_hrev32_i32() for 32-bit values,
extract tcg_gen_hrev64_i64() for 64-bit ones.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 docs/devel/tcg-ops.rst      |  4 +++-
 include/tcg/tcg-op-common.h |  1 +
 tcg/tcg-op.c                | 29 +++++++++++++++++++++++------
 3 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/docs/devel/tcg-ops.rst b/docs/devel/tcg-ops.rst
index 17965faa03..e8a2f8aacc 100644
--- a/docs/devel/tcg-ops.rst
+++ b/docs/devel/tcg-ops.rst
@@ -492,7 +492,9 @@ Misc
 
    * - hrev32_i32 *t0*, *t1*
 
-     - | Byteswap each halfword within a 32-bit value.
+       hrev64_i64 *t0*, *t1*
+
+     - | Byteswap each halfword within a 32/64-bit value.
 
    * - hswap_i32/i64 *t0*, *t1*
 
diff --git a/include/tcg/tcg-op-common.h b/include/tcg/tcg-op-common.h
index bb515dfd51..a9184caf9d 100644
--- a/include/tcg/tcg-op-common.h
+++ b/include/tcg/tcg-op-common.h
@@ -562,6 +562,7 @@ void tcg_gen_bswap32_i64(TCGv_i64 ret, TCGv_i64 arg, int flags);
 void tcg_gen_bswap64_i64(TCGv_i64 ret, TCGv_i64 arg);
 void tcg_gen_hswap_i64(TCGv_i64 ret, TCGv_i64 arg);
 void tcg_gen_wswap_i64(TCGv_i64 ret, TCGv_i64 arg);
+void tcg_gen_hrev64_i64(TCGv_i64 ret, TCGv_i64 arg);
 void tcg_gen_smin_i64(TCGv_i64, TCGv_i64 arg1, TCGv_i64 arg2);
 void tcg_gen_smax_i64(TCGv_i64, TCGv_i64 arg1, TCGv_i64 arg2);
 void tcg_gen_umin_i64(TCGv_i64, TCGv_i64 arg1, TCGv_i64 arg2);
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index b1b5d9b45b..310acce410 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -1876,12 +1876,7 @@ void tcg_gen_bswap64_i64(TCGv_i64 ret, TCGv_i64 arg)
         TCGv_i64 t2 = tcg_temp_ebb_new_i64();
 
                                         /* arg = abcdefgh */
-        tcg_gen_movi_i64(t2, 0x00ff00ff00ff00ffull);
-        tcg_gen_shri_i64(t0, arg, 8);   /*  t0 = .abcdefg */
-        tcg_gen_and_i64(t1, arg, t2);   /*  t1 = .b.d.f.h */
-        tcg_gen_and_i64(t0, t0, t2);    /*  t0 = .a.c.e.g */
-        tcg_gen_shli_i64(t1, t1, 8);    /*  t1 = b.d.f.h. */
-        tcg_gen_or_i64(ret, t0, t1);    /* ret = badcfehg */
+        tcg_gen_hrev64_i64(ret, arg);   /* ret = badcfehg */
 
         tcg_gen_movi_i64(t2, 0x0000ffff0000ffffull);
         tcg_gen_shri_i64(t0, ret, 16);  /*  t0 = ..badcfe */
@@ -1936,6 +1931,28 @@ void tcg_gen_wswap_i64(TCGv_i64 ret, TCGv_i64 arg)
     tcg_gen_rotli_i64(ret, arg, 32);
 }
 
+/*
+ * hrev64_i64: Byteswap each halfwords within a 64-bit value.
+ *
+ * Byte pattern:  hrev64_i64(abcdefgh) -> badcfehg
+ */
+void tcg_gen_hrev64_i64(TCGv_i64 ret, TCGv_i64 arg)
+{
+    TCGv_i64 mask = tcg_constant_i64(0x00ff00ff00ff00ffull);
+    TCGv_i64 t1 = tcg_temp_ebb_new_i64();
+    TCGv_i64 t2 = tcg_temp_ebb_new_i64();
+
+                                        /* arg = abcdefgh */
+    tcg_gen_shri_i64(t1, arg, 8);       /*  t1 = .abcdefg */
+    tcg_gen_and_i64(t2, t1, mask);      /*  t2 = .a.c.e.g */
+    tcg_gen_and_i64(t1, arg, mask);     /*  t1 = .b.d.f.h */
+    tcg_gen_shli_i64(t1, t1, 8);        /*  t1 = b.d.f.h. */
+    tcg_gen_or_i64(ret, t1, t2);        /* ret = badcfehg */
+
+    tcg_temp_free_i64(t1);
+    tcg_temp_free_i64(t2);
+}
+
 void tcg_gen_not_i64(TCGv_i64 ret, TCGv_i64 arg)
 {
     if (TCG_TARGET_REG_BITS == 32) {
-- 
2.41.0

[PATCH 10/12] target/arm: Use generic hrev_i64() in Aarch64 REV16 opcode

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/tcg/translate-a64.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 5fa1257d32..2973831b38 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -5040,16 +5040,7 @@ static void handle_rev32(DisasContext *s, unsigned int sf,
 static void handle_rev16(DisasContext *s, unsigned int sf,
                          unsigned int rn, unsigned int rd)
 {
-    TCGv_i64 tcg_rd = cpu_reg(s, rd);
-    TCGv_i64 tcg_tmp = tcg_temp_new_i64();
-    TCGv_i64 tcg_rn = read_cpu_reg(s, rn, sf);
-    TCGv_i64 mask = tcg_constant_i64(sf ? 0x00ff00ff00ff00ffull : 0x00ff00ff);
-
-    tcg_gen_shri_i64(tcg_tmp, tcg_rn, 8);
-    tcg_gen_and_i64(tcg_rd, tcg_rn, mask);
-    tcg_gen_and_i64(tcg_tmp, tcg_tmp, mask);
-    tcg_gen_shli_i64(tcg_rd, tcg_rd, 8);
-    tcg_gen_or_i64(tcg_rd, tcg_rd, tcg_tmp);
+    tcg_gen_hrev_i64(cpu_reg(s, rd), read_cpu_reg(s, rn, sf), sf);
 }
 
 /* Data-processing (1 source)
-- 
2.41.0

[PATCH 09/12] tcg/tcg-op: Add tcg_gen_hrev32_i64() and tcg_gen_hrev_i64()

[Philippe Mathieu-Daudé]

tcg_gen_hrev32_i64() is similar to tcg_gen_hrev64_i64() but
only modifies the lower 32-bit of a 64-bit value.

tcg_gen_hrev_i64() can be used when we don't know at build
time whether to clear the 32 high bits of the value or not.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 docs/devel/tcg-ops.rst      |  4 +++
 include/tcg/tcg-op-common.h |  2 ++
 include/tcg/tcg-op.h        |  2 ++
 tcg/tcg-op.c                | 49 +++++++++++++++++++++++++++----------
 4 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/docs/devel/tcg-ops.rst b/docs/devel/tcg-ops.rst
index e8a2f8aacc..3a8104929c 100644
--- a/docs/devel/tcg-ops.rst
+++ b/docs/devel/tcg-ops.rst
@@ -496,6 +496,10 @@ Misc
 
      - | Byteswap each halfword within a 32/64-bit value.
 
+   * - hrev32_i64 *t0*, *t1*
+
+     - | Byteswap each halfword on the low bits of a 64-bit value.
+
    * - hswap_i32/i64 *t0*, *t1*
 
      - | Swap 16-bit halfwords within a 32/64-bit value.
diff --git a/include/tcg/tcg-op-common.h b/include/tcg/tcg-op-common.h
index a9184caf9d..eb327ed964 100644
--- a/include/tcg/tcg-op-common.h
+++ b/include/tcg/tcg-op-common.h
@@ -562,7 +562,9 @@ void tcg_gen_bswap32_i64(TCGv_i64 ret, TCGv_i64 arg, int flags);
 void tcg_gen_bswap64_i64(TCGv_i64 ret, TCGv_i64 arg);
 void tcg_gen_hswap_i64(TCGv_i64 ret, TCGv_i64 arg);
 void tcg_gen_wswap_i64(TCGv_i64 ret, TCGv_i64 arg);
+void tcg_gen_hrev32_i64(TCGv_i64 ret, TCGv_i64 arg);
 void tcg_gen_hrev64_i64(TCGv_i64 ret, TCGv_i64 arg);
+void tcg_gen_hrev_i64(TCGv_i64 ret, TCGv_i64 arg, int is64);
 void tcg_gen_smin_i64(TCGv_i64, TCGv_i64 arg1, TCGv_i64 arg2);
 void tcg_gen_smax_i64(TCGv_i64, TCGv_i64 arg1, TCGv_i64 arg2);
 void tcg_gen_umin_i64(TCGv_i64, TCGv_i64 arg1, TCGv_i64 arg2);
diff --git a/include/tcg/tcg-op.h b/include/tcg/tcg-op.h
index d63683c47b..3ac1d13b19 100644
--- a/include/tcg/tcg-op.h
+++ b/include/tcg/tcg-op.h
@@ -225,6 +225,7 @@ DEF_ATOMIC2(tcg_gen_atomic_umax_fetch, i64)
 #define tcg_gen_bswap_tl tcg_gen_bswap64_i64
 #define tcg_gen_hswap_tl tcg_gen_hswap_i64
 #define tcg_gen_wswap_tl tcg_gen_wswap_i64
+#define tcg_gen_hrev32_tl tcg_gen_hrev32_i64
 #define tcg_gen_concat_tl_i64 tcg_gen_concat32_i64
 #define tcg_gen_extr_i64_tl tcg_gen_extr32_i64
 #define tcg_gen_andc_tl tcg_gen_andc_i64
@@ -340,6 +341,7 @@ DEF_ATOMIC2(tcg_gen_atomic_umax_fetch, i64)
 #define tcg_gen_bswap32_tl(D, S, F) tcg_gen_bswap32_i32(D, S)
 #define tcg_gen_bswap_tl tcg_gen_bswap32_i32
 #define tcg_gen_hswap_tl tcg_gen_hswap_i32
+#define tcg_gen_hrev32_tl tcg_gen_hrev32_i32
 #define tcg_gen_concat_tl_i64 tcg_gen_concat_i32_i64
 #define tcg_gen_extr_i64_tl tcg_gen_extr_i64_i32
 #define tcg_gen_andc_tl tcg_gen_andc_i32
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index 310acce410..75892e91ef 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -1931,6 +1931,41 @@ void tcg_gen_wswap_i64(TCGv_i64 ret, TCGv_i64 arg)
     tcg_gen_rotli_i64(ret, arg, 32);
 }
 
+/*
+ * hrev_i64: Byteswap each halfwords within a 64-bit value.
+ *           If %is64 is not set, the 32 high bits are zeroed.
+ *
+ * Byte pattern:  hrev_i64(xxxxabcd, 0) -> ....badc
+ *                hrev_i64(abcdefgh, 1) -> badcfehg
+ */
+void tcg_gen_hrev_i64(TCGv_i64 ret, TCGv_i64 arg, int is64)
+{
+    TCGv_i64 mask = tcg_constant_i64(is64 ? 0x00ff00ff00ff00ffull : 0x00ff00ff);
+    TCGv_i64 t1 = tcg_temp_ebb_new_i64();
+    TCGv_i64 t2 = tcg_temp_ebb_new_i64();
+
+                                        /*        is64=0      is64=1  */
+                                        /* arg = xxxxabcd    abcdefgh */
+    tcg_gen_shri_i64(t1, arg, 8);       /*  t1 = .xxxxabc    .abcdefg */
+    tcg_gen_and_i64(t2, t1, mask);      /*  t2 = .....a.c    .a.c.e.g */
+    tcg_gen_and_i64(t1, arg, mask);     /*  t1 = .....b.d    .b.d.f.h */
+    tcg_gen_shli_i64(t1, t1, 8);        /*  t1 = ....b.d.    b.d.f.h. */
+    tcg_gen_or_i64(ret, t1, t2);        /* ret = ....badc    badcfehg */
+
+    tcg_temp_free_i64(t1);
+    tcg_temp_free_i64(t2);
+}
+
+/*
+ * hrev32_i64: Byteswap each halfword on the low bits of a 64-bit value.
+ *
+ * Byte pattern:  hrev32_i64(xxxxabcd) -> ....badc
+ */
+void tcg_gen_hrev32_i64(TCGv_i64 ret, TCGv_i64 arg)
+{
+    tcg_gen_hrev_i64(ret, arg, false);
+}
+
 /*
  * hrev64_i64: Byteswap each halfwords within a 64-bit value.
  *
@@ -1938,19 +1973,7 @@ void tcg_gen_wswap_i64(TCGv_i64 ret, TCGv_i64 arg)
  */
 void tcg_gen_hrev64_i64(TCGv_i64 ret, TCGv_i64 arg)
 {
-    TCGv_i64 mask = tcg_constant_i64(0x00ff00ff00ff00ffull);
-    TCGv_i64 t1 = tcg_temp_ebb_new_i64();
-    TCGv_i64 t2 = tcg_temp_ebb_new_i64();
-
-                                        /* arg = abcdefgh */
-    tcg_gen_shri_i64(t1, arg, 8);       /*  t1 = .abcdefg */
-    tcg_gen_and_i64(t2, t1, mask);      /*  t2 = .a.c.e.g */
-    tcg_gen_and_i64(t1, arg, mask);     /*  t1 = .b.d.f.h */
-    tcg_gen_shli_i64(t1, t1, 8);        /*  t1 = b.d.f.h. */
-    tcg_gen_or_i64(ret, t1, t2);        /* ret = badcfehg */
-
-    tcg_temp_free_i64(t1);
-    tcg_temp_free_i64(t2);
+    tcg_gen_hrev_i64(ret, arg, true);
 }
 
 void tcg_gen_not_i64(TCGv_i64 ret, TCGv_i64 arg)
-- 
2.41.0

[PATCH 08/12] target/loongarch: Use generic hrev64_i64() in REVB.4H opcode

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/loongarch/insn_trans/trans_bit.c.inc | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/target/loongarch/insn_trans/trans_bit.c.inc b/target/loongarch/insn_trans/trans_bit.c.inc
index 95b4ee5bc8..c04806dc21 100644
--- a/target/loongarch/insn_trans/trans_bit.c.inc
+++ b/target/loongarch/insn_trans/trans_bit.c.inc
@@ -124,19 +124,6 @@ static void gen_revb_2h(TCGv dest, TCGv src1)
     tcg_gen_or_tl(dest, t0, t1);
 }
 
-static void gen_revb_4h(TCGv dest, TCGv src1)
-{
-    TCGv mask = tcg_constant_tl(0x00FF00FF00FF00FFULL);
-    TCGv t0 = tcg_temp_new();
-    TCGv t1 = tcg_temp_new();
-
-    tcg_gen_shri_tl(t0, src1, 8);
-    tcg_gen_and_tl(t0, t0, mask);
-    tcg_gen_and_tl(t1, src1, mask);
-    tcg_gen_shli_tl(t1, t1, 8);
-    tcg_gen_or_tl(dest, t0, t1);
-}
-
 static void gen_revh_2w(TCGv dest, TCGv src1)
 {
     TCGv_i64 t0 = tcg_temp_new_i64();
@@ -175,7 +162,7 @@ TRANS(clz_d, gen_rr, EXT_NONE, EXT_NONE, gen_clz_d)
 TRANS(cto_d, gen_rr, EXT_NONE, EXT_NONE, gen_cto_d)
 TRANS(ctz_d, gen_rr, EXT_NONE, EXT_NONE, gen_ctz_d)
 TRANS(revb_2h, gen_rr, EXT_NONE, EXT_SIGN, gen_revb_2h)
-TRANS(revb_4h, gen_rr, EXT_NONE, EXT_NONE, gen_revb_4h)
+TRANS(revb_4h, gen_rr, EXT_NONE, EXT_NONE, tcg_gen_hrev64_i64)
 TRANS(revb_2w, gen_rr, EXT_NONE, EXT_NONE, gen_revb_2w)
 TRANS(revb_d, gen_rr, EXT_NONE, EXT_NONE, tcg_gen_bswap64_i64)
 TRANS(revh_2w, gen_rr, EXT_NONE, EXT_NONE, gen_revh_2w)
-- 
2.41.0

Re: [PATCH 08/12] target/loongarch: Use generic hrev64_i64() in REVB.4H opcode

[Philippe Mathieu-Daudé]

On 22/8/23 14:51, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/loongarch/insn_trans/trans_bit.c.inc | 15 +--------------
>   1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/target/loongarch/insn_trans/trans_bit.c.inc b/target/loongarch/insn_trans/trans_bit.c.inc
> index 95b4ee5bc8..c04806dc21 100644
> --- a/target/loongarch/insn_trans/trans_bit.c.inc
> +++ b/target/loongarch/insn_trans/trans_bit.c.inc
> @@ -124,19 +124,6 @@ static void gen_revb_2h(TCGv dest, TCGv src1)
>       tcg_gen_or_tl(dest, t0, t1);
>   }
>   
> -static void gen_revb_4h(TCGv dest, TCGv src1)
> -{
> -    TCGv mask = tcg_constant_tl(0x00FF00FF00FF00FFULL);
> -    TCGv t0 = tcg_temp_new();
> -    TCGv t1 = tcg_temp_new();
> -
> -    tcg_gen_shri_tl(t0, src1, 8);
> -    tcg_gen_and_tl(t0, t0, mask);
> -    tcg_gen_and_tl(t1, src1, mask);
> -    tcg_gen_shli_tl(t1, t1, 8);
> -    tcg_gen_or_tl(dest, t0, t1);
> -}
> -
>   static void gen_revh_2w(TCGv dest, TCGv src1)
>   {
>       TCGv_i64 t0 = tcg_temp_new_i64();
> @@ -175,7 +162,7 @@ TRANS(clz_d, gen_rr, EXT_NONE, EXT_NONE, gen_clz_d)
>   TRANS(cto_d, gen_rr, EXT_NONE, EXT_NONE, gen_cto_d)
>   TRANS(ctz_d, gen_rr, EXT_NONE, EXT_NONE, gen_ctz_d)
>   TRANS(revb_2h, gen_rr, EXT_NONE, EXT_SIGN, gen_revb_2h)
> -TRANS(revb_4h, gen_rr, EXT_NONE, EXT_NONE, gen_revb_4h)
> +TRANS(revb_4h, gen_rr, EXT_NONE, EXT_NONE, tcg_gen_hrev64_i64)

We should use tcg_gen_hrev64_tl() instead.

Re: [PATCH 08/12] target/loongarch: Use generic hrev64_i64() in REVB.4H opcode

[Philippe Mathieu-Daudé]

On 22/8/23 15:28, Philippe Mathieu-Daudé wrote:
> On 22/8/23 14:51, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/loongarch/insn_trans/trans_bit.c.inc | 15 +--------------
>>   1 file changed, 1 insertion(+), 14 deletions(-)
>>
>> diff --git a/target/loongarch/insn_trans/trans_bit.c.inc 
>> b/target/loongarch/insn_trans/trans_bit.c.inc
>> index 95b4ee5bc8..c04806dc21 100644
>> --- a/target/loongarch/insn_trans/trans_bit.c.inc
>> +++ b/target/loongarch/insn_trans/trans_bit.c.inc
>> @@ -124,19 +124,6 @@ static void gen_revb_2h(TCGv dest, TCGv src1)
>>       tcg_gen_or_tl(dest, t0, t1);
>>   }
>> -static void gen_revb_4h(TCGv dest, TCGv src1)
>> -{
>> -    TCGv mask = tcg_constant_tl(0x00FF00FF00FF00FFULL);
>> -    TCGv t0 = tcg_temp_new();
>> -    TCGv t1 = tcg_temp_new();
>> -
>> -    tcg_gen_shri_tl(t0, src1, 8);
>> -    tcg_gen_and_tl(t0, t0, mask);
>> -    tcg_gen_and_tl(t1, src1, mask);
>> -    tcg_gen_shli_tl(t1, t1, 8);
>> -    tcg_gen_or_tl(dest, t0, t1);
>> -}
>> -
>>   static void gen_revh_2w(TCGv dest, TCGv src1)
>>   {
>>       TCGv_i64 t0 = tcg_temp_new_i64();
>> @@ -175,7 +162,7 @@ TRANS(clz_d, gen_rr, EXT_NONE, EXT_NONE, gen_clz_d)
>>   TRANS(cto_d, gen_rr, EXT_NONE, EXT_NONE, gen_cto_d)
>>   TRANS(ctz_d, gen_rr, EXT_NONE, EXT_NONE, gen_ctz_d)
>>   TRANS(revb_2h, gen_rr, EXT_NONE, EXT_SIGN, gen_revb_2h)
>> -TRANS(revb_4h, gen_rr, EXT_NONE, EXT_NONE, gen_revb_4h)
>> +TRANS(revb_4h, gen_rr, EXT_NONE, EXT_NONE, tcg_gen_hrev64_i64)
> 
> We should use tcg_gen_hrev64_tl() instead.

Although due to the 0x00FF00FF00FF00FFULL constant it
seems gen_revb_4h() really expects i64 registers...
(see gen_revh_2w which uses TCGv_i64).

I suppose gen_revb_4h() ended that way as a copy/paste of
gen_revb_2h().

[PATCH 11/12] target/loongarch: Use generic hrev64_i32() in REVB.2H opcode

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/loongarch/insn_trans/trans_bit.c.inc | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/target/loongarch/insn_trans/trans_bit.c.inc b/target/loongarch/insn_trans/trans_bit.c.inc
index c04806dc21..9d564a0999 100644
--- a/target/loongarch/insn_trans/trans_bit.c.inc
+++ b/target/loongarch/insn_trans/trans_bit.c.inc
@@ -111,19 +111,6 @@ static void gen_revb_2w(TCGv dest, TCGv src1)
     tcg_gen_rotri_i64(dest, dest, 32);
 }
 
-static void gen_revb_2h(TCGv dest, TCGv src1)
-{
-    TCGv mask = tcg_constant_tl(0x00FF00FF);
-    TCGv t0 = tcg_temp_new();
-    TCGv t1 = tcg_temp_new();
-
-    tcg_gen_shri_tl(t0, src1, 8);
-    tcg_gen_and_tl(t0, t0, mask);
-    tcg_gen_and_tl(t1, src1, mask);
-    tcg_gen_shli_tl(t1, t1, 8);
-    tcg_gen_or_tl(dest, t0, t1);
-}
-
 static void gen_revh_2w(TCGv dest, TCGv src1)
 {
     TCGv_i64 t0 = tcg_temp_new_i64();
@@ -161,7 +148,7 @@ TRANS(clo_d, gen_rr, EXT_NONE, EXT_NONE, gen_clo_d)
 TRANS(clz_d, gen_rr, EXT_NONE, EXT_NONE, gen_clz_d)
 TRANS(cto_d, gen_rr, EXT_NONE, EXT_NONE, gen_cto_d)
 TRANS(ctz_d, gen_rr, EXT_NONE, EXT_NONE, gen_ctz_d)
-TRANS(revb_2h, gen_rr, EXT_NONE, EXT_SIGN, gen_revb_2h)
+TRANS(revb_2h, gen_rr, EXT_NONE, EXT_SIGN, tcg_gen_hrev32_i64)
 TRANS(revb_4h, gen_rr, EXT_NONE, EXT_NONE, tcg_gen_hrev64_i64)
 TRANS(revb_2w, gen_rr, EXT_NONE, EXT_NONE, gen_revb_2w)
 TRANS(revb_d, gen_rr, EXT_NONE, EXT_NONE, tcg_gen_bswap64_i64)
-- 
2.41.0

Re: [PATCH 11/12] target/loongarch: Use generic hrev64_i32() in REVB.2H opcode

[Philippe Mathieu-Daudé]

On 22/8/23 14:52, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/loongarch/insn_trans/trans_bit.c.inc | 15 +--------------
>   1 file changed, 1 insertion(+), 14 deletions(-)
> 
> diff --git a/target/loongarch/insn_trans/trans_bit.c.inc b/target/loongarch/insn_trans/trans_bit.c.inc
> index c04806dc21..9d564a0999 100644
> --- a/target/loongarch/insn_trans/trans_bit.c.inc
> +++ b/target/loongarch/insn_trans/trans_bit.c.inc
> @@ -111,19 +111,6 @@ static void gen_revb_2w(TCGv dest, TCGv src1)
>       tcg_gen_rotri_i64(dest, dest, 32);
>   }
>   
> -static void gen_revb_2h(TCGv dest, TCGv src1)
> -{
> -    TCGv mask = tcg_constant_tl(0x00FF00FF);
> -    TCGv t0 = tcg_temp_new();
> -    TCGv t1 = tcg_temp_new();
> -
> -    tcg_gen_shri_tl(t0, src1, 8);
> -    tcg_gen_and_tl(t0, t0, mask);
> -    tcg_gen_and_tl(t1, src1, mask);
> -    tcg_gen_shli_tl(t1, t1, 8);
> -    tcg_gen_or_tl(dest, t0, t1);
> -}
> -
>   static void gen_revh_2w(TCGv dest, TCGv src1)
>   {
>       TCGv_i64 t0 = tcg_temp_new_i64();
> @@ -161,7 +148,7 @@ TRANS(clo_d, gen_rr, EXT_NONE, EXT_NONE, gen_clo_d)
>   TRANS(clz_d, gen_rr, EXT_NONE, EXT_NONE, gen_clz_d)
>   TRANS(cto_d, gen_rr, EXT_NONE, EXT_NONE, gen_cto_d)
>   TRANS(ctz_d, gen_rr, EXT_NONE, EXT_NONE, gen_ctz_d)
> -TRANS(revb_2h, gen_rr, EXT_NONE, EXT_SIGN, gen_revb_2h)
> +TRANS(revb_2h, gen_rr, EXT_NONE, EXT_SIGN, tcg_gen_hrev32_i64)

While this file uses a mix of _i64/_tl (so likely doesn't build
for 32-bit target), we should use tcg_gen_hrev32_tl() I suppose...

>   TRANS(revb_4h, gen_rr, EXT_NONE, EXT_NONE, tcg_gen_hrev64_i64)
>   TRANS(revb_2w, gen_rr, EXT_NONE, EXT_NONE, gen_revb_2w)
>   TRANS(revb_d, gen_rr, EXT_NONE, EXT_NONE, tcg_gen_bswap64_i64)

Re: [PATCH 11/12] target/loongarch: Use generic hrev64_i32() in REVB.2H opcode

[Philippe Mathieu-Daudé]

On 22/8/23 15:30, Philippe Mathieu-Daudé wrote:
> On 22/8/23 14:52, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/loongarch/insn_trans/trans_bit.c.inc | 15 +--------------
>>   1 file changed, 1 insertion(+), 14 deletions(-)
>>
>> diff --git a/target/loongarch/insn_trans/trans_bit.c.inc 
>> b/target/loongarch/insn_trans/trans_bit.c.inc
>> index c04806dc21..9d564a0999 100644
>> --- a/target/loongarch/insn_trans/trans_bit.c.inc
>> +++ b/target/loongarch/insn_trans/trans_bit.c.inc
>> @@ -111,19 +111,6 @@ static void gen_revb_2w(TCGv dest, TCGv src1)
>>       tcg_gen_rotri_i64(dest, dest, 32);
>>   }
>> -static void gen_revb_2h(TCGv dest, TCGv src1)
>> -{
>> -    TCGv mask = tcg_constant_tl(0x00FF00FF);
>> -    TCGv t0 = tcg_temp_new();
>> -    TCGv t1 = tcg_temp_new();
>> -
>> -    tcg_gen_shri_tl(t0, src1, 8);
>> -    tcg_gen_and_tl(t0, t0, mask);
>> -    tcg_gen_and_tl(t1, src1, mask);
>> -    tcg_gen_shli_tl(t1, t1, 8);

Looking at 
https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#_revb_2h4h2wd
the sign extension is missing, so the next line:

>> -    tcg_gen_or_tl(dest, t0, t1);

should be replaced by smth like:

         tcg_gen_or_tl(t0, t0, t1);
         tcg_gen_ext32s_tl(dest, t0);

>> -}

Re: [PATCH 11/12] target/loongarch: Use generic hrev64_i32() in REVB.2H opcode

[Richard Henderson]

On 8/22/23 09:11, Philippe Mathieu-Daudé wrote:
>>> -static void gen_revb_2h(TCGv dest, TCGv src1)
>>> -{
>>> -    TCGv mask = tcg_constant_tl(0x00FF00FF);
>>> -    TCGv t0 = tcg_temp_new();
>>> -    TCGv t1 = tcg_temp_new();
>>> -
>>> -    tcg_gen_shri_tl(t0, src1, 8);
>>> -    tcg_gen_and_tl(t0, t0, mask);
>>> -    tcg_gen_and_tl(t1, src1, mask);
>>> -    tcg_gen_shli_tl(t1, t1, 8);
> 
> Looking at 
> https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#_revb_2h4h2wd
> the sign extension is missing, so the next line:
> 
>>> -    tcg_gen_or_tl(dest, t0, t1);
> 
> should be replaced by smth like:
> 
>          tcg_gen_or_tl(t0, t0, t1);
>          tcg_gen_ext32s_tl(dest, t0);

Extension is handled by

TRANS(revb_2h, gen_rr, EXT_NONE, EXT_SIGN, gen_revb_2h)
                                  ^^^^^^^^^
this


r~

[PATCH 07/12] target/ppc: Use generic hrev64_i64() in BRH / BSWAP16x8 opcodes

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/ppc/translate.c              | 10 +---------
 target/ppc/translate/vsx-impl.c.inc | 19 ++-----------------
 2 files changed, 3 insertions(+), 26 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 74796ec7ba..91a9ec2d1c 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6435,15 +6435,7 @@ static void gen_brw(DisasContext *ctx)
 /* brh */
 static void gen_brh(DisasContext *ctx)
 {
-    TCGv_i64 mask = tcg_constant_i64(0x00ff00ff00ff00ffull);
-    TCGv_i64 t1 = tcg_temp_new_i64();
-    TCGv_i64 t2 = tcg_temp_new_i64();
-
-    tcg_gen_shri_i64(t1, cpu_gpr[rS(ctx->opcode)], 8);
-    tcg_gen_and_i64(t2, t1, mask);
-    tcg_gen_and_i64(t1, cpu_gpr[rS(ctx->opcode)], mask);
-    tcg_gen_shli_i64(t1, t1, 8);
-    tcg_gen_or_i64(cpu_gpr[rA(ctx->opcode)], t1, t2);
+    tcg_gen_hrev64_i64(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
 }
 #endif
 
diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
index 0f5b0056f1..639ab7f1bc 100644
--- a/target/ppc/translate/vsx-impl.c.inc
+++ b/target/ppc/translate/vsx-impl.c.inc
@@ -154,23 +154,8 @@ static void gen_lxvdsx(DisasContext *ctx)
 static void gen_bswap16x8(TCGv_i64 outh, TCGv_i64 outl,
                           TCGv_i64 inh, TCGv_i64 inl)
 {
-    TCGv_i64 mask = tcg_constant_i64(0x00FF00FF00FF00FF);
-    TCGv_i64 t0 = tcg_temp_new_i64();
-    TCGv_i64 t1 = tcg_temp_new_i64();
-
-    /* outh = ((inh & mask) << 8) | ((inh >> 8) & mask) */
-    tcg_gen_and_i64(t0, inh, mask);
-    tcg_gen_shli_i64(t0, t0, 8);
-    tcg_gen_shri_i64(t1, inh, 8);
-    tcg_gen_and_i64(t1, t1, mask);
-    tcg_gen_or_i64(outh, t0, t1);
-
-    /* outl = ((inl & mask) << 8) | ((inl >> 8) & mask) */
-    tcg_gen_and_i64(t0, inl, mask);
-    tcg_gen_shli_i64(t0, t0, 8);
-    tcg_gen_shri_i64(t1, inl, 8);
-    tcg_gen_and_i64(t1, t1, mask);
-    tcg_gen_or_i64(outl, t0, t1);
+    tcg_gen_hrev64_i64(outh, inh);
+    tcg_gen_hrev64_i64(outl, inl);
 }
 
 static void gen_bswap32x4(TCGv_i64 outh, TCGv_i64 outl,
-- 
2.41.0

Re: [PATCH 07/12] target/ppc: Use generic hrev64_i64() in BRH / BSWAP16x8 opcodes

[Daniel Henrique Barboza]

On 8/22/23 09:53, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   target/ppc/translate.c              | 10 +---------
>   target/ppc/translate/vsx-impl.c.inc | 19 ++-----------------
>   2 files changed, 3 insertions(+), 26 deletions(-)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 74796ec7ba..91a9ec2d1c 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6435,15 +6435,7 @@ static void gen_brw(DisasContext *ctx)
>   /* brh */
>   static void gen_brh(DisasContext *ctx)
>   {
> -    TCGv_i64 mask = tcg_constant_i64(0x00ff00ff00ff00ffull);
> -    TCGv_i64 t1 = tcg_temp_new_i64();
> -    TCGv_i64 t2 = tcg_temp_new_i64();
> -
> -    tcg_gen_shri_i64(t1, cpu_gpr[rS(ctx->opcode)], 8);
> -    tcg_gen_and_i64(t2, t1, mask);
> -    tcg_gen_and_i64(t1, cpu_gpr[rS(ctx->opcode)], mask);
> -    tcg_gen_shli_i64(t1, t1, 8);
> -    tcg_gen_or_i64(cpu_gpr[rA(ctx->opcode)], t1, t2);
> +    tcg_gen_hrev64_i64(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
>   }
>   #endif
>   
> diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
> index 0f5b0056f1..639ab7f1bc 100644
> --- a/target/ppc/translate/vsx-impl.c.inc
> +++ b/target/ppc/translate/vsx-impl.c.inc
> @@ -154,23 +154,8 @@ static void gen_lxvdsx(DisasContext *ctx)
>   static void gen_bswap16x8(TCGv_i64 outh, TCGv_i64 outl,
>                             TCGv_i64 inh, TCGv_i64 inl)
>   {
> -    TCGv_i64 mask = tcg_constant_i64(0x00FF00FF00FF00FF);
> -    TCGv_i64 t0 = tcg_temp_new_i64();
> -    TCGv_i64 t1 = tcg_temp_new_i64();
> -
> -    /* outh = ((inh & mask) << 8) | ((inh >> 8) & mask) */
> -    tcg_gen_and_i64(t0, inh, mask);
> -    tcg_gen_shli_i64(t0, t0, 8);
> -    tcg_gen_shri_i64(t1, inh, 8);
> -    tcg_gen_and_i64(t1, t1, mask);
> -    tcg_gen_or_i64(outh, t0, t1);
> -
> -    /* outl = ((inl & mask) << 8) | ((inl >> 8) & mask) */
> -    tcg_gen_and_i64(t0, inl, mask);
> -    tcg_gen_shli_i64(t0, t0, 8);
> -    tcg_gen_shri_i64(t1, inl, 8);
> -    tcg_gen_and_i64(t1, t1, mask);
> -    tcg_gen_or_i64(outl, t0, t1);
> +    tcg_gen_hrev64_i64(outh, inh);
> +    tcg_gen_hrev64_i64(outl, inl);
>   }
>   
>   static void gen_bswap32x4(TCGv_i64 outh, TCGv_i64 outl,

Re: [PATCH 07/12] target/ppc: Use generic hrev64_i64() in BRH / BSWAP16x8 opcodes

[Nicholas Piggin]

On Tue Aug 22, 2023 at 10:53 PM AEST, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/ppc/translate.c              | 10 +---------
>  target/ppc/translate/vsx-impl.c.inc | 19 ++-----------------
>  2 files changed, 3 insertions(+), 26 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 74796ec7ba..91a9ec2d1c 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6435,15 +6435,7 @@ static void gen_brw(DisasContext *ctx)
>  /* brh */
>  static void gen_brh(DisasContext *ctx)
>  {
> -    TCGv_i64 mask = tcg_constant_i64(0x00ff00ff00ff00ffull);
> -    TCGv_i64 t1 = tcg_temp_new_i64();
> -    TCGv_i64 t2 = tcg_temp_new_i64();
> -
> -    tcg_gen_shri_i64(t1, cpu_gpr[rS(ctx->opcode)], 8);
> -    tcg_gen_and_i64(t2, t1, mask);
> -    tcg_gen_and_i64(t1, cpu_gpr[rS(ctx->opcode)], mask);
> -    tcg_gen_shli_i64(t1, t1, 8);
> -    tcg_gen_or_i64(cpu_gpr[rA(ctx->opcode)], t1, t2);
> +    tcg_gen_hrev64_i64(cpu_gpr[rA(ctx->opcode)], cpu_gpr[rS(ctx->opcode)]);
>  }
>  #endif
>  
> diff --git a/target/ppc/translate/vsx-impl.c.inc b/target/ppc/translate/vsx-impl.c.inc
> index 0f5b0056f1..639ab7f1bc 100644
> --- a/target/ppc/translate/vsx-impl.c.inc
> +++ b/target/ppc/translate/vsx-impl.c.inc
> @@ -154,23 +154,8 @@ static void gen_lxvdsx(DisasContext *ctx)
>  static void gen_bswap16x8(TCGv_i64 outh, TCGv_i64 outl,
>                            TCGv_i64 inh, TCGv_i64 inl)
>  {
> -    TCGv_i64 mask = tcg_constant_i64(0x00FF00FF00FF00FF);
> -    TCGv_i64 t0 = tcg_temp_new_i64();
> -    TCGv_i64 t1 = tcg_temp_new_i64();
> -
> -    /* outh = ((inh & mask) << 8) | ((inh >> 8) & mask) */
> -    tcg_gen_and_i64(t0, inh, mask);
> -    tcg_gen_shli_i64(t0, t0, 8);
> -    tcg_gen_shri_i64(t1, inh, 8);
> -    tcg_gen_and_i64(t1, t1, mask);
> -    tcg_gen_or_i64(outh, t0, t1);
> -
> -    /* outl = ((inl & mask) << 8) | ((inl >> 8) & mask) */
> -    tcg_gen_and_i64(t0, inl, mask);
> -    tcg_gen_shli_i64(t0, t0, 8);
> -    tcg_gen_shri_i64(t1, inl, 8);
> -    tcg_gen_and_i64(t1, t1, mask);
> -    tcg_gen_or_i64(outl, t0, t1);
> +    tcg_gen_hrev64_i64(outh, inh);
> +    tcg_gen_hrev64_i64(outl, inl);
>  }
>  
>  static void gen_bswap32x4(TCGv_i64 outh, TCGv_i64 outl,

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

[PATCH 06/12] target/mips: Use generic hrev64_i64() in DSBH opcode

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/tcg/translate.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 4f34ea9b6a..08ee745a6d 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -4936,16 +4936,7 @@ static void gen_bshfl(DisasContext *ctx, uint32_t op2, int rt, int rd)
         break;
 #if defined(TARGET_MIPS64)
     case OPC_DSBH:
-        {
-            TCGv t1 = tcg_temp_new();
-            TCGv t2 = tcg_constant_tl(0x00FF00FF00FF00FFULL);
-
-            tcg_gen_shri_tl(t1, t0, 8);
-            tcg_gen_and_tl(t1, t1, t2);
-            tcg_gen_and_tl(t0, t0, t2);
-            tcg_gen_shli_tl(t0, t0, 8);
-            tcg_gen_or_tl(cpu_gpr[rd], t0, t1);
-        }
+        tcg_gen_hrev64_i64(cpu_gpr[rd], t0);
         break;
     case OPC_DSHD:
         tcg_gen_hswap_i64(cpu_gpr[rd], t0);
-- 
2.41.0

[PATCH 12/12] target/mips: Use generic hrev32_tl() in WSBH opcode

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/mips/tcg/translate.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 08ee745a6d..822b932262 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -4916,17 +4916,8 @@ static void gen_bshfl(DisasContext *ctx, uint32_t op2, int rt, int rd)
     gen_load_gpr(t0, rt);
     switch (op2) {
     case OPC_WSBH:
-        {
-            TCGv t1 = tcg_temp_new();
-            TCGv t2 = tcg_constant_tl(0x00FF00FF);
-
-            tcg_gen_shri_tl(t1, t0, 8);
-            tcg_gen_and_tl(t1, t1, t2);
-            tcg_gen_and_tl(t0, t0, t2);
-            tcg_gen_shli_tl(t0, t0, 8);
-            tcg_gen_or_tl(t0, t0, t1);
-            tcg_gen_ext32s_tl(cpu_gpr[rd], t0);
-        }
+        tcg_gen_hrev32_tl(t0, t0);
+        tcg_gen_ext32s_tl(cpu_gpr[rd], t0);
         break;
     case OPC_SEB:
         tcg_gen_ext8s_tl(cpu_gpr[rd], t0);
-- 
2.41.0

Re: [PATCH 12/12] target/mips: Use generic hrev32_tl() in WSBH opcode

[Richard Henderson]

On 8/22/23 05:57, Philippe Mathieu-Daudé wrote:
> -        {
> -            TCGv t1 = tcg_temp_new();
> -            TCGv t2 = tcg_constant_tl(0x00FF00FF);
> -
> -            tcg_gen_shri_tl(t1, t0, 8);
> -            tcg_gen_and_tl(t1, t1, t2);
> -            tcg_gen_and_tl(t0, t0, t2);
> -            tcg_gen_shli_tl(t0, t0, 8);
> -            tcg_gen_or_tl(t0, t0, t1);
> -            tcg_gen_ext32s_tl(cpu_gpr[rd], t0);
> -        }
> +        tcg_gen_hrev32_tl(t0, t0);
> +        tcg_gen_ext32s_tl(cpu_gpr[rd], t0);

Ah, you did want tcg_gen_bswap16x2_tl(dst, src, TCG_BSWAP_OS).


r~

Re: [PATCH 12/12] target/mips: Use generic hrev32_tl() in WSBH opcode

[Philippe Mathieu-Daudé]

On 22/8/23 17:42, Richard Henderson wrote:
> On 8/22/23 05:57, Philippe Mathieu-Daudé wrote:
>> -        {
>> -            TCGv t1 = tcg_temp_new();
>> -            TCGv t2 = tcg_constant_tl(0x00FF00FF);
>> -
>> -            tcg_gen_shri_tl(t1, t0, 8);
>> -            tcg_gen_and_tl(t1, t1, t2);
>> -            tcg_gen_and_tl(t0, t0, t2);
>> -            tcg_gen_shli_tl(t0, t0, 8);
>> -            tcg_gen_or_tl(t0, t0, t1);
>> -            tcg_gen_ext32s_tl(cpu_gpr[rd], t0);
>> -        }
>> +        tcg_gen_hrev32_tl(t0, t0);
>> +        tcg_gen_ext32s_tl(cpu_gpr[rd], t0);
> 
> Ah, you did want tcg_gen_bswap16x2_tl(dst, src, TCG_BSWAP_OS).

Got it (per your reply to the cover).

> 
> 
> r~

Re: [PATCH 00/12] tcg: Factor hrev{32,64}_{i32,i64,tl} out

[Richard Henderson]

On 8/22/23 05:40, Philippe Mathieu-Daudé wrote:
> This series factor the "byteswap each halfword within a
> 32/64-bit value" code duplication as generic helpers.
> 
> Modulo the documentation added, there is a good negative
> diff-stat, so I believe this is a win from a maintainance
> point of view.
> 
> I used "hrev" to follow the other bswap/hswap/rev helpers
> but it isn't very descriptive, so any better name suggestion
> is welcomed.
> (In particular because there are other patterns I'd like to
> factor out and then naming is getting worse, such 'wrev').'

I applaud the code factor, but the names are poor.

The "h" does not match the size of the elements being swapped, which is "b".  The "32" 
is... what, the total count of bits modified?

Naming is hard, and I'm not sure what's best.

We have bswap32_i32, bswap32_i64, bswap64_i64.

Perhaps bswap16x2_i32, bswap16x4_i64, bswap16xN_tl, to indicate that we're bswaping 16-bit 
quantities, and "xN" to indicate that multiple 16-bit quantities are being swapped.

 From your subjects, it would appear we don't need bswap16x2_i64, with the upper 32-bits 
zero/signed/undefined.  But if we did, we should provide a flags argument of TCG_BSWAP_*.

That then extends to hswap32x2_i64 to swap halfwords within multiple words for mips DSHW 
et al.


r~

Re: [PATCH 00/12] tcg: Factor hrev{32,64}_{i32,i64,tl} out

[Nicholas Piggin]

On Tue Aug 22, 2023 at 10:40 PM AEST, Philippe Mathieu-Daudé wrote:
> This series factor the "byteswap each halfword within a
> 32/64-bit value" code duplication as generic helpers.
>
> Modulo the documentation added, there is a good negative
> diff-stat, so I believe this is a win from a maintainance
> point of view.
>
> I used "hrev" to follow the other bswap/hswap/rev helpers
> but it isn't very descriptive, so any better name suggestion
> is welcomed.
> (In particular because there are other patterns I'd like to
> factor out and then naming is getting worse, such 'wrev').

I know bswap has alrady precedent, but you could drop the bit
size when it matches the operand size, hrev_i64.

Could possibly also follow ppc and call it brevh, which tells
you the units being swapped and the size they are swapped in.
Then you could add brevw or hrevw etc. and it might be a bit
less ambiguous.

Looks like a nice cleanup though.

Thanks,
Nick

>
> Philippe Mathieu-Daudé (12):
>   tcg/tcg-op: Factor tcg_gen_hrev32_i32() out
>   target/arm: Use generic hrev32_i32() in ARM REV16 and VREV16 opcodes
>   target/cris: Use generic hrev32_i32() in SWAPB opcode
>   target/rx: Use generic hrev32_i32() in REVW opcode
>   tcg/tcg-op: Factor tcg_gen_hrev64_i64() out
>   target/mips: Use generic hrev64_i64() in DSBH opcode
>   target/ppc: Use generic hrev64_i64() in BRH / BSWAP16x8 opcodes
>   target/loongarch: Use generic hrev64_i64() in REVB.4H opcode
>   tcg/tcg-op: Add tcg_gen_hrev32_i64() and tcg_gen_hrev_i64()
>   target/arm: Use generic hrev_i64() in Aarch64 REV16 opcode
>   target/loongarch: Use generic hrev64_i32() in REVB.2H opcode
>   target/mips: Use generic hrev32_tl() in WSBH opcode
>
>  docs/devel/tcg-ops.rst                      | 10 +++
>  include/tcg/tcg-op-common.h                 |  4 +
>  include/tcg/tcg-op.h                        |  2 +
>  target/arm/tcg/translate-a32.h              |  1 -
>  target/arm/tcg/translate-a64.c              | 11 +--
>  target/arm/tcg/translate-neon.c             |  2 +-
>  target/arm/tcg/translate.c                  | 14 +---
>  target/cris/translate.c                     | 20 +----
>  target/mips/tcg/translate.c                 | 24 +-----
>  target/ppc/translate.c                      | 10 +--
>  target/rx/translate.c                       |  8 +-
>  tcg/tcg-op.c                                | 81 ++++++++++++++++++---
>  target/cris/translate_v10.c.inc             |  2 +-
>  target/loongarch/insn_trans/trans_bit.c.inc | 30 +-------
>  target/ppc/translate/vsx-impl.c.inc         | 19 +----
>  15 files changed, 99 insertions(+), 139 deletions(-)