[PATCH 0/6] target: Use TCG generic gen_hswap_i32/i64()

[PATCH 0/6] target: Use TCG generic gen_hswap_i32/i64()

[Philippe Mathieu-Daudé]

Some targets open-code gen_hswap();
replace by the generic helper.

Philippe Mathieu-Daudé (6):
  target/arm: Use hswap_i32() in VREV/SMLAD opcodes
  target/cris: Use hswap_i32() in SWAPW opcode
  target/microblaze: Use hswap_i32() in SWAPH opcode
  target/sh4: Use hswap_i32() in SWAP.W opcode
  target/mips: Use hswap_i64() in DSHD opcode
  target/loongarch: Use hswap_i64() in REVH.D opcode

 target/arm/tcg/translate-a32.h              |  6 ------
 target/arm/tcg/translate-neon.c             |  4 ++--
 target/arm/tcg/translate.c                  |  4 ++--
 target/cris/translate.c                     | 14 +-------------
 target/microblaze/translate.c               |  7 +------
 target/mips/tcg/translate.c                 | 14 +-------------
 target/sh4/translate.c                      |  2 +-
 target/cris/translate_v10.c.inc             |  2 +-
 target/loongarch/insn_trans/trans_bit.c.inc | 16 +---------------
 9 files changed, 10 insertions(+), 59 deletions(-)

-- 
2.41.0

[PATCH 1/6] target/arm: Use hswap_i32() in VREV/SMLAD opcodes

[Philippe Mathieu-Daudé]

Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
introduced the generic hswap_i32(). Use it instead of open-coding
it as gen_swap_half().

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

diff --git a/target/arm/tcg/translate-a32.h b/target/arm/tcg/translate-a32.h
index 48a15379d2..0c8f408eea 100644
--- a/target/arm/tcg/translate-a32.h
+++ b/target/arm/tcg/translate-a32.h
@@ -158,10 +158,4 @@ DO_GEN_ST(32, MO_UL)
 /* Set NZCV flags from the high 4 bits of var.  */
 #define gen_set_nzcv(var) gen_set_cpsr(var, CPSR_NZCV)
 
-/* Swap low and high halfwords.  */
-static inline void gen_swap_half(TCGv_i32 dest, TCGv_i32 var)
-{
-    tcg_gen_rotri_i32(dest, var, 16);
-}
-
 #endif
diff --git a/target/arm/tcg/translate-neon.c b/target/arm/tcg/translate-neon.c
index 8de4ceb203..0e59b03ff9 100644
--- a/target/arm/tcg/translate-neon.c
+++ b/target/arm/tcg/translate-neon.c
@@ -2906,7 +2906,7 @@ static bool trans_VREV64(DisasContext *s, arg_VREV64 *a)
                 tcg_gen_bswap32_i32(tmp[half], tmp[half]);
                 break;
             case 1:
-                gen_swap_half(tmp[half], tmp[half]);
+                tcg_gen_hswap_i32(tmp[half], tmp[half]);
                 break;
             case 2:
                 break;
@@ -3516,7 +3516,7 @@ static bool trans_VREV32(DisasContext *s, arg_2misc *a)
 {
     static NeonGenOneOpFn * const fn[] = {
         tcg_gen_bswap32_i32,
-        gen_swap_half,
+        tcg_gen_hswap_i32,
         NULL,
         NULL,
     };
diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
index 1a6938d1b3..39a42611c6 100644
--- a/target/arm/tcg/translate.c
+++ b/target/arm/tcg/translate.c
@@ -7612,7 +7612,7 @@ static bool op_smlad(DisasContext *s, arg_rrrr *a, bool m_swap, bool sub)
     t1 = load_reg(s, a->rn);
     t2 = load_reg(s, a->rm);
     if (m_swap) {
-        gen_swap_half(t2, t2);
+        tcg_gen_hswap_i32(t2, t2);
     }
     gen_smul_dual(t1, t2);
 
@@ -7700,7 +7700,7 @@ static bool op_smlald(DisasContext *s, arg_rrrr *a, bool m_swap, bool sub)
     t1 = load_reg(s, a->rn);
     t2 = load_reg(s, a->rm);
     if (m_swap) {
-        gen_swap_half(t2, t2);
+        tcg_gen_hswap_i32(t2, t2);
     }
     gen_smul_dual(t1, t2);
 
-- 
2.41.0

[PATCH 2/6] target/cris: Use hswap_i32() in SWAPW opcode

[Philippe Mathieu-Daudé]

Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
introduced the generic hswap_i32(). Use it instead of open-coding
it as t_gen_swapw().

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

diff --git a/target/cris/translate.c b/target/cris/translate.c
index 42103b5558..925ed2c6f6 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -399,18 +399,6 @@ static inline void t_gen_swapb(TCGv d, TCGv s)
     tcg_gen_or_tl(d, d, t);
 }
 
-/* Swap the halfwords of the s operand.  */
-static inline void t_gen_swapw(TCGv d, TCGv s)
-{
-    TCGv t;
-    /* d and s refer the same object.  */
-    t = tcg_temp_new();
-    tcg_gen_mov_tl(t, s);
-    tcg_gen_shli_tl(d, t, 16);
-    tcg_gen_shri_tl(t, t, 16);
-    tcg_gen_or_tl(d, d, t);
-}
-
 /*
  * Reverse the bits within each byte.
  *
@@ -1675,7 +1663,7 @@ static int dec_swap_r(CPUCRISState *env, DisasContext *dc)
         tcg_gen_not_tl(t0, t0);
     }
     if (dc->op2 & 4) {
-        t_gen_swapw(t0, t0);
+        tcg_gen_hswap_i32(t0, t0);
     }
     if (dc->op2 & 2) {
         t_gen_swapb(t0, t0);
diff --git a/target/cris/translate_v10.c.inc b/target/cris/translate_v10.c.inc
index b7b0517982..0ff15769ec 100644
--- a/target/cris/translate_v10.c.inc
+++ b/target/cris/translate_v10.c.inc
@@ -506,7 +506,7 @@ static void dec10_reg_swap(DisasContext *dc)
     if (dc->dst & 8)
         tcg_gen_not_tl(t0, t0);
     if (dc->dst & 4)
-        t_gen_swapw(t0, t0);
+        tcg_gen_hswap_i32(t0, t0);
     if (dc->dst & 2)
         t_gen_swapb(t0, t0);
     if (dc->dst & 1)
-- 
2.41.0

[PATCH 3/6] target/microblaze: Use hswap_i32() in SWAPH opcode

[Philippe Mathieu-Daudé]

Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
introduced the generic hswap_i32(). Use it instead of open-coding
it as gen_swaph().

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

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 7e7f837c63..83fd1161f0 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -608,13 +608,8 @@ DO_TYPEA0(sra, false, gen_sra)
 DO_TYPEA0(src, false, gen_src)
 DO_TYPEA0(srl, false, gen_srl)
 
-static void gen_swaph(TCGv_i32 out, TCGv_i32 ina)
-{
-    tcg_gen_rotri_i32(out, ina, 16);
-}
-
 DO_TYPEA0(swapb, false, tcg_gen_bswap32_i32)
-DO_TYPEA0(swaph, false, gen_swaph)
+DO_TYPEA0(swaph, false, tcg_gen_hswap_i32)
 
 static bool trans_wdic(DisasContext *dc, arg_wdic *a)
 {
-- 
2.41.0

[PATCH 4/6] target/sh4: Use hswap_i32() in SWAP.W opcode

[Philippe Mathieu-Daudé]

Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
introduced the generic hswap_i32(). Use it.

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

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 49c87d7a01..0f96a099b3 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -678,7 +678,7 @@ static void _decode_opc(DisasContext * ctx)
 	}
 	return;
     case 0x6009:		/* swap.w Rm,Rn */
-        tcg_gen_rotli_i32(REG(B11_8), REG(B7_4), 16);
+        tcg_gen_hswap_i32(REG(B11_8), REG(B7_4));
 	return;
     case 0x200d:		/* xtrct Rm,Rn */
 	{
-- 
2.41.0

[PATCH 5/6] target/mips: Use hswap_i64() in DSHD opcode

[Philippe Mathieu-Daudé]

Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
introduced the generic hswap_i32(). Use it instead of open-coding
it in gen_bshfl().

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

diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 9bb40f1849..4f34ea9b6a 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -4948,19 +4948,7 @@ static void gen_bshfl(DisasContext *ctx, uint32_t op2, int rt, int rd)
         }
         break;
     case OPC_DSHD:
-        {
-            TCGv t1 = tcg_temp_new();
-            TCGv t2 = tcg_constant_tl(0x0000FFFF0000FFFFULL);
-
-            tcg_gen_shri_tl(t1, t0, 16);
-            tcg_gen_and_tl(t1, t1, t2);
-            tcg_gen_and_tl(t0, t0, t2);
-            tcg_gen_shli_tl(t0, t0, 16);
-            tcg_gen_or_tl(t0, t0, t1);
-            tcg_gen_shri_tl(t1, t0, 32);
-            tcg_gen_shli_tl(t0, t0, 32);
-            tcg_gen_or_tl(cpu_gpr[rd], t0, t1);
-        }
+        tcg_gen_hswap_i64(cpu_gpr[rd], t0);
         break;
 #endif
     default:
-- 
2.41.0

[PATCH 6/6] target/loongarch: Use hswap_i64() in REVH.D opcode

[Philippe Mathieu-Daudé]

Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
introduced the generic hswap_i32(). Use it instead of open-coding
it as gen_revh_d().

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

diff --git a/target/loongarch/insn_trans/trans_bit.c.inc b/target/loongarch/insn_trans/trans_bit.c.inc
index 25b4d7858b..95b4ee5bc8 100644
--- a/target/loongarch/insn_trans/trans_bit.c.inc
+++ b/target/loongarch/insn_trans/trans_bit.c.inc
@@ -150,20 +150,6 @@ static void gen_revh_2w(TCGv dest, TCGv src1)
     tcg_gen_or_i64(dest, t1, t0);
 }
 
-static void gen_revh_d(TCGv dest, TCGv src1)
-{
-    TCGv t0 = tcg_temp_new();
-    TCGv t1 = tcg_temp_new();
-    TCGv mask = tcg_constant_tl(0x0000FFFF0000FFFFULL);
-
-    tcg_gen_shri_tl(t1, src1, 16);
-    tcg_gen_and_tl(t1, t1, mask);
-    tcg_gen_and_tl(t0, src1, mask);
-    tcg_gen_shli_tl(t0, t0, 16);
-    tcg_gen_or_tl(t0, t0, t1);
-    tcg_gen_rotri_tl(dest, t0, 32);
-}
-
 static void gen_maskeqz(TCGv dest, TCGv src1, TCGv src2)
 {
     TCGv zero = tcg_constant_tl(0);
@@ -193,7 +179,7 @@ TRANS(revb_4h, gen_rr, EXT_NONE, EXT_NONE, gen_revb_4h)
 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)
-TRANS(revh_d, gen_rr, EXT_NONE, EXT_NONE, gen_revh_d)
+TRANS(revh_d, gen_rr, EXT_NONE, EXT_NONE, tcg_gen_hswap_i64)
 TRANS(bitrev_4b, gen_rr, EXT_ZERO, EXT_SIGN, gen_helper_bitswap)
 TRANS(bitrev_8b, gen_rr, EXT_NONE, EXT_NONE, gen_helper_bitswap)
 TRANS(bitrev_w, gen_rr, EXT_NONE, EXT_SIGN, gen_helper_bitrev_w)
-- 
2.41.0

Re: [PATCH 2/6] target/cris: Use hswap_i32() in SWAPW opcode

[Peter Maydell]

On Tue, 22 Aug 2023 at 12:01, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
> introduced the generic hswap_i32(). Use it instead of open-coding
> it as t_gen_swapw().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/cris/translate.c         | 14 +-------------
>  target/cris/translate_v10.c.inc |  2 +-
>  2 files changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/target/cris/translate.c b/target/cris/translate.c
> index 42103b5558..925ed2c6f6 100644
> --- a/target/cris/translate.c
> +++ b/target/cris/translate.c
> @@ -399,18 +399,6 @@ static inline void t_gen_swapb(TCGv d, TCGv s)
>      tcg_gen_or_tl(d, d, t);
>  }
>
> -/* Swap the halfwords of the s operand.  */
> -static inline void t_gen_swapw(TCGv d, TCGv s)
> -{
> -    TCGv t;
> -    /* d and s refer the same object.  */
> -    t = tcg_temp_new();
> -    tcg_gen_mov_tl(t, s);
> -    tcg_gen_shli_tl(d, t, 16);
> -    tcg_gen_shri_tl(t, t, 16);
> -    tcg_gen_or_tl(d, d, t);
> -}
> -
>  /*
>   * Reverse the bits within each byte.
>   *
> @@ -1675,7 +1663,7 @@ static int dec_swap_r(CPUCRISState *env, DisasContext *dc)
>          tcg_gen_not_tl(t0, t0);
>      }
>      if (dc->op2 & 4) {
> -        t_gen_swapw(t0, t0);
> +        tcg_gen_hswap_i32(t0, t0);
>      }
>      if (dc->op2 & 2) {
>          t_gen_swapb(t0, t0);
> diff --git a/target/cris/translate_v10.c.inc b/target/cris/translate_v10.c.inc
> index b7b0517982..0ff15769ec 100644
> --- a/target/cris/translate_v10.c.inc
> +++ b/target/cris/translate_v10.c.inc
> @@ -506,7 +506,7 @@ static void dec10_reg_swap(DisasContext *dc)
>      if (dc->dst & 8)
>          tcg_gen_not_tl(t0, t0);
>      if (dc->dst & 4)
> -        t_gen_swapw(t0, t0);
> +        tcg_gen_hswap_i32(t0, t0);

Both these are operating on TCGv, not TCGv_i32, so I think this
should be tcg_gen_hswap_tl(). (Compare the tcg_gen_not_tl()
calls.)

>      if (dc->dst & 2)
>          t_gen_swapb(t0, t0);
>      if (dc->dst & 1)

thanks
-- PMM

Re: [PATCH 5/6] target/mips: Use hswap_i64() in DSHD opcode

[Peter Maydell]

On Tue, 22 Aug 2023 at 12:02, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
> introduced the generic hswap_i32(). Use it instead of open-coding
> it in gen_bshfl().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/mips/tcg/translate.c | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
> index 9bb40f1849..4f34ea9b6a 100644
> --- a/target/mips/tcg/translate.c
> +++ b/target/mips/tcg/translate.c
> @@ -4948,19 +4948,7 @@ static void gen_bshfl(DisasContext *ctx, uint32_t op2, int rt, int rd)
>          }
>          break;
>      case OPC_DSHD:
> -        {
> -            TCGv t1 = tcg_temp_new();
> -            TCGv t2 = tcg_constant_tl(0x0000FFFF0000FFFFULL);
> -
> -            tcg_gen_shri_tl(t1, t0, 16);
> -            tcg_gen_and_tl(t1, t1, t2);
> -            tcg_gen_and_tl(t0, t0, t2);
> -            tcg_gen_shli_tl(t0, t0, 16);
> -            tcg_gen_or_tl(t0, t0, t1);
> -            tcg_gen_shri_tl(t1, t0, 32);
> -            tcg_gen_shli_tl(t0, t0, 32);
> -            tcg_gen_or_tl(cpu_gpr[rd], t0, t1);
> -        }
> +        tcg_gen_hswap_i64(cpu_gpr[rd], t0);

We know that target_long here is 64 bits, but for consistency
with the rest of the function maybe tcg_gen_hswap_tl()
is better? I'm not sure...

thanks
-- PMM

Re: [PATCH 2/6] target/cris: Use hswap_i32() in SWAPW opcode

[Philippe Mathieu-Daudé]

On 22/8/23 13:44, Peter Maydell wrote:
> On Tue, 22 Aug 2023 at 12:01, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
>> introduced the generic hswap_i32(). Use it instead of open-coding
>> it as t_gen_swapw().
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/cris/translate.c         | 14 +-------------
>>   target/cris/translate_v10.c.inc |  2 +-
>>   2 files changed, 2 insertions(+), 14 deletions(-)


>> diff --git a/target/cris/translate_v10.c.inc b/target/cris/translate_v10.c.inc
>> index b7b0517982..0ff15769ec 100644
>> --- a/target/cris/translate_v10.c.inc
>> +++ b/target/cris/translate_v10.c.inc
>> @@ -506,7 +506,7 @@ static void dec10_reg_swap(DisasContext *dc)
>>       if (dc->dst & 8)
>>           tcg_gen_not_tl(t0, t0);
>>       if (dc->dst & 4)
>> -        t_gen_swapw(t0, t0);
>> +        tcg_gen_hswap_i32(t0, t0);
> 
> Both these are operating on TCGv, not TCGv_i32, so I think this
> should be tcg_gen_hswap_tl(). (Compare the tcg_gen_not_tl()
> calls.)

You are correct, if someone copies part of this code to a new
function compiled for a 64-bit target, this won't build.

We know cris is a 32-bit only target.

When implementing tcg_gen_foo_tl(), should we implement both
corresponding tcg_gen_foo_i32/i64() even if one is never used
(thus not tested)?

I like completeness, but I'm a bit reluctant to commit unused
code (mostly for maintenance burden).

Maybe I can go mid-way and only add tcg_gen_hswap_tl() ->
tcg_gen_hswap_i32() here. If tcg_gen_hswap_tl() were used on
a 64-bit target then we'd get a build failure. Does that
sound reasonable?

Thanks,

Phil.

Re: [PATCH 2/6] target/cris: Use hswap_i32() in SWAPW opcode

[Peter Maydell]

On Tue, 22 Aug 2023 at 14:06, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 22/8/23 13:44, Peter Maydell wrote:
> > On Tue, 22 Aug 2023 at 12:01, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> >>
> >> Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
> >> introduced the generic hswap_i32(). Use it instead of open-coding
> >> it as t_gen_swapw().
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >> ---
> >>   target/cris/translate.c         | 14 +-------------
> >>   target/cris/translate_v10.c.inc |  2 +-
> >>   2 files changed, 2 insertions(+), 14 deletions(-)
>
>
> >> diff --git a/target/cris/translate_v10.c.inc b/target/cris/translate_v10.c.inc
> >> index b7b0517982..0ff15769ec 100644
> >> --- a/target/cris/translate_v10.c.inc
> >> +++ b/target/cris/translate_v10.c.inc
> >> @@ -506,7 +506,7 @@ static void dec10_reg_swap(DisasContext *dc)
> >>       if (dc->dst & 8)
> >>           tcg_gen_not_tl(t0, t0);
> >>       if (dc->dst & 4)
> >> -        t_gen_swapw(t0, t0);
> >> +        tcg_gen_hswap_i32(t0, t0);
> >
> > Both these are operating on TCGv, not TCGv_i32, so I think this
> > should be tcg_gen_hswap_tl(). (Compare the tcg_gen_not_tl()
> > calls.)
>
> You are correct, if someone copies part of this code to a new
> function compiled for a 64-bit target, this won't build.
>
> We know cris is a 32-bit only target.
>
> When implementing tcg_gen_foo_tl(), should we implement both
> corresponding tcg_gen_foo_i32/i64() even if one is never used
> (thus not tested)?
>
> I like completeness, but I'm a bit reluctant to commit unused
> code (mostly for maintenance burden).
>
> Maybe I can go mid-way and only add tcg_gen_hswap_tl() ->
> tcg_gen_hswap_i32() here. If tcg_gen_hswap_tl() were used on
> a 64-bit target then we'd get a build failure. Does that
> sound reasonable?

We already have tcg_gen_hswap_tl (it's a #define like all the
_tl symbols), so I'm just asking that you use it rather than
the _i32 version. If we were writing the cris target code
from scratch these days we'd probably write it to use _i32
throughout, but since it's not written that way I think
it's better to continue the pattern rather than deviate
from it.

thanks
-- PMM

Re: [PATCH 2/6] target/cris: Use hswap_i32() in SWAPW opcode

[Philippe Mathieu-Daudé]

On 22/8/23 15:27, Peter Maydell wrote:
> On Tue, 22 Aug 2023 at 14:06, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 22/8/23 13:44, Peter Maydell wrote:
>>> On Tue, 22 Aug 2023 at 12:01, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>>>
>>>> Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
>>>> introduced the generic hswap_i32(). Use it instead of open-coding
>>>> it as t_gen_swapw().
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>>    target/cris/translate.c         | 14 +-------------
>>>>    target/cris/translate_v10.c.inc |  2 +-
>>>>    2 files changed, 2 insertions(+), 14 deletions(-)
>>
>>
>>>> diff --git a/target/cris/translate_v10.c.inc b/target/cris/translate_v10.c.inc
>>>> index b7b0517982..0ff15769ec 100644
>>>> --- a/target/cris/translate_v10.c.inc
>>>> +++ b/target/cris/translate_v10.c.inc
>>>> @@ -506,7 +506,7 @@ static void dec10_reg_swap(DisasContext *dc)
>>>>        if (dc->dst & 8)
>>>>            tcg_gen_not_tl(t0, t0);
>>>>        if (dc->dst & 4)
>>>> -        t_gen_swapw(t0, t0);
>>>> +        tcg_gen_hswap_i32(t0, t0);
>>>
>>> Both these are operating on TCGv, not TCGv_i32, so I think this
>>> should be tcg_gen_hswap_tl(). (Compare the tcg_gen_not_tl()
>>> calls.)
>>
>> You are correct, if someone copies part of this code to a new
>> function compiled for a 64-bit target, this won't build.
>>
>> We know cris is a 32-bit only target.
>>
>> When implementing tcg_gen_foo_tl(), should we implement both
>> corresponding tcg_gen_foo_i32/i64() even if one is never used
>> (thus not tested)?
>>
>> I like completeness, but I'm a bit reluctant to commit unused
>> code (mostly for maintenance burden).
>>
>> Maybe I can go mid-way and only add tcg_gen_hswap_tl() ->
>> tcg_gen_hswap_i32() here. If tcg_gen_hswap_tl() were used on
>> a 64-bit target then we'd get a build failure. Does that
>> sound reasonable?
> 
> We already have tcg_gen_hswap_tl (it's a #define like all the
> _tl symbols), so I'm just asking that you use it rather than
> the _i32 version. If we were writing the cris target code
> from scratch these days we'd probably write it to use _i32
> throughout, but since it's not written that way I think
> it's better to continue the pattern rather than deviate
> from it.

Doh I missed commit 46be8425ff also added tcg_gen_hswap_tl()...

Thanks!

Phil.

Re: [PATCH 1/6] target/arm: Use hswap_i32() in VREV/SMLAD opcodes

[Richard Henderson]

On 8/22/23 04:01, Philippe Mathieu-Daudé wrote:
> Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
> introduced the generic hswap_i32(). Use it instead of open-coding
> it as gen_swap_half().
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/arm/tcg/translate-a32.h  | 6 ------
>   target/arm/tcg/translate-neon.c | 4 ++--
>   target/arm/tcg/translate.c      | 4 ++--
>   3 files changed, 4 insertions(+), 10 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 3/6] target/microblaze: Use hswap_i32() in SWAPH opcode

[Richard Henderson]

On 8/22/23 04:01, Philippe Mathieu-Daudé wrote:
> Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
> introduced the generic hswap_i32(). Use it instead of open-coding
> it as gen_swaph().
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/microblaze/translate.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 4/6] target/sh4: Use hswap_i32() in SWAP.W opcode

[Richard Henderson]

On 8/22/23 04:01, Philippe Mathieu-Daudé wrote:
> Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
> introduced the generic hswap_i32(). Use it.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/sh4/translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 6/6] target/loongarch: Use hswap_i64() in REVH.D opcode

[Richard Henderson]

On 8/22/23 04:01, Philippe Mathieu-Daudé wrote:
> Commit 46be8425ff ("tcg: Implement tcg_gen_{h,w}swap_{i32,i64}")
> introduced the generic hswap_i32(). Use it instead of open-coding
> it as gen_revh_d().
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/loongarch/insn_trans/trans_bit.c.inc | 16 +---------------
>   1 file changed, 1 insertion(+), 15 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~