[PATCH 0/7] tcg: Expose tcg_gen_ext_{i32,i64,tl}

[PATCH 0/7] tcg: Expose tcg_gen_ext_{i32,i64,tl}

[Richard Henderson]

We have several copies of this function (or equivalent).
Expose the one already present in tcg-op-ldst.c.
Then, some target cleanups while searching for "ext8[us]".


r~


Richard Henderson (7):
  tcg: Export tcg_gen_ext_{i32,i64,tl}
  target/arm: Use tcg_gen_ext_i64
  target/i386: Use tcg_gen_ext_tl
  target/m68k: Use tcg_gen_ext_i32
  target/rx: Use tcg_gen_ext_i32
  target/tricore: Use tcg_gen_*extract_tl
  target/xtensa: Use tcg_gen_sextract_i32

 include/tcg/tcg-op-common.h    |  2 ++
 include/tcg/tcg-op.h           |  2 ++
 target/arm/tcg/translate-a64.c | 37 ++--------------------------------
 target/i386/tcg/translate.c    | 28 +++----------------------
 target/m68k/translate.c        | 23 +++------------------
 target/rx/translate.c          | 11 +++-------
 target/tricore/translate.c     | 20 ++++--------------
 target/xtensa/translate.c      | 12 +----------
 tcg/tcg-op-ldst.c              | 14 +++++++++----
 9 files changed, 30 insertions(+), 119 deletions(-)

-- 
2.34.1

[PATCH 1/7] tcg: Export tcg_gen_ext_{i32,i64,tl}

[Richard Henderson]

The two concrete type functions already existed, merely needing
a bit of hardening to invalid inputs.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/tcg/tcg-op-common.h |  2 ++
 include/tcg/tcg-op.h        |  2 ++
 tcg/tcg-op-ldst.c           | 14 ++++++++++----
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/tcg/tcg-op-common.h b/include/tcg/tcg-op-common.h
index a0bae5df01..677aea6dd1 100644
--- a/include/tcg/tcg-op-common.h
+++ b/include/tcg/tcg-op-common.h
@@ -361,6 +361,7 @@ void tcg_gen_ext8s_i32(TCGv_i32 ret, TCGv_i32 arg);
 void tcg_gen_ext16s_i32(TCGv_i32 ret, TCGv_i32 arg);
 void tcg_gen_ext8u_i32(TCGv_i32 ret, TCGv_i32 arg);
 void tcg_gen_ext16u_i32(TCGv_i32 ret, TCGv_i32 arg);
+void tcg_gen_ext_i32(TCGv_i32 ret, TCGv_i32 val, MemOp opc);
 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);
@@ -564,6 +565,7 @@ void tcg_gen_ext32s_i64(TCGv_i64 ret, TCGv_i64 arg);
 void tcg_gen_ext8u_i64(TCGv_i64 ret, TCGv_i64 arg);
 void tcg_gen_ext16u_i64(TCGv_i64 ret, TCGv_i64 arg);
 void tcg_gen_ext32u_i64(TCGv_i64 ret, TCGv_i64 arg);
+void tcg_gen_ext_i64(TCGv_i64 ret, TCGv_i64 val, MemOp opc);
 void tcg_gen_bswap16_i64(TCGv_i64 ret, TCGv_i64 arg, int flags);
 void tcg_gen_bswap32_i64(TCGv_i64 ret, TCGv_i64 arg, int flags);
 void tcg_gen_bswap64_i64(TCGv_i64 ret, TCGv_i64 arg);
diff --git a/include/tcg/tcg-op.h b/include/tcg/tcg-op.h
index e81dd7dd9e..a02850583b 100644
--- a/include/tcg/tcg-op.h
+++ b/include/tcg/tcg-op.h
@@ -219,6 +219,7 @@ DEF_ATOMIC2(tcg_gen_atomic_umax_fetch, i64)
 #define tcg_gen_ext16s_tl tcg_gen_ext16s_i64
 #define tcg_gen_ext32u_tl tcg_gen_ext32u_i64
 #define tcg_gen_ext32s_tl tcg_gen_ext32s_i64
+#define tcg_gen_ext_tl tcg_gen_ext_i64
 #define tcg_gen_bswap16_tl tcg_gen_bswap16_i64
 #define tcg_gen_bswap32_tl tcg_gen_bswap32_i64
 #define tcg_gen_bswap64_tl tcg_gen_bswap64_i64
@@ -338,6 +339,7 @@ DEF_ATOMIC2(tcg_gen_atomic_umax_fetch, i64)
 #define tcg_gen_ext16s_tl tcg_gen_ext16s_i32
 #define tcg_gen_ext32u_tl tcg_gen_mov_i32
 #define tcg_gen_ext32s_tl tcg_gen_mov_i32
+#define tcg_gen_ext_tl tcg_gen_ext_i32
 #define tcg_gen_bswap16_tl tcg_gen_bswap16_i32
 #define tcg_gen_bswap32_tl(D, S, F) tcg_gen_bswap32_i32(D, S)
 #define tcg_gen_bswap_tl tcg_gen_bswap32_i32
diff --git a/tcg/tcg-op-ldst.c b/tcg/tcg-op-ldst.c
index 2b96687699..e2c55df217 100644
--- a/tcg/tcg-op-ldst.c
+++ b/tcg/tcg-op-ldst.c
@@ -714,7 +714,7 @@ void tcg_gen_qemu_st_i128_chk(TCGv_i128 val, TCGTemp *addr, TCGArg idx,
     tcg_gen_qemu_st_i128_int(val, addr, idx, memop);
 }
 
-static void tcg_gen_ext_i32(TCGv_i32 ret, TCGv_i32 val, MemOp opc)
+void tcg_gen_ext_i32(TCGv_i32 ret, TCGv_i32 val, MemOp opc)
 {
     switch (opc & MO_SSIZE) {
     case MO_SB:
@@ -729,13 +729,16 @@ static void tcg_gen_ext_i32(TCGv_i32 ret, TCGv_i32 val, MemOp opc)
     case MO_UW:
         tcg_gen_ext16u_i32(ret, val);
         break;
-    default:
+    case MO_UL:
+    case MO_SL:
         tcg_gen_mov_i32(ret, val);
         break;
+    default:
+        g_assert_not_reached();
     }
 }
 
-static void tcg_gen_ext_i64(TCGv_i64 ret, TCGv_i64 val, MemOp opc)
+void tcg_gen_ext_i64(TCGv_i64 ret, TCGv_i64 val, MemOp opc)
 {
     switch (opc & MO_SSIZE) {
     case MO_SB:
@@ -756,9 +759,12 @@ static void tcg_gen_ext_i64(TCGv_i64 ret, TCGv_i64 val, MemOp opc)
     case MO_UL:
         tcg_gen_ext32u_i64(ret, val);
         break;
-    default:
+    case MO_UQ:
+    case MO_SQ:
         tcg_gen_mov_i64(ret, val);
         break;
+    default:
+        g_assert_not_reached();
     }
 }
 
-- 
2.34.1

[PATCH 2/7] target/arm: Use tcg_gen_ext_i64

[Richard Henderson]

The ext_and_shift_reg helper does this plus a shift.
The non-zero check for shift count is duplicate to
the one done within tcg_gen_shli_i64.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/tcg/translate-a64.c | 37 ++--------------------------------
 1 file changed, 2 insertions(+), 35 deletions(-)

diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 10e8dcf743..ad78b8b120 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -1324,41 +1324,8 @@ static void ext_and_shift_reg(TCGv_i64 tcg_out, TCGv_i64 tcg_in,
     int extsize = extract32(option, 0, 2);
     bool is_signed = extract32(option, 2, 1);
 
-    if (is_signed) {
-        switch (extsize) {
-        case 0:
-            tcg_gen_ext8s_i64(tcg_out, tcg_in);
-            break;
-        case 1:
-            tcg_gen_ext16s_i64(tcg_out, tcg_in);
-            break;
-        case 2:
-            tcg_gen_ext32s_i64(tcg_out, tcg_in);
-            break;
-        case 3:
-            tcg_gen_mov_i64(tcg_out, tcg_in);
-            break;
-        }
-    } else {
-        switch (extsize) {
-        case 0:
-            tcg_gen_ext8u_i64(tcg_out, tcg_in);
-            break;
-        case 1:
-            tcg_gen_ext16u_i64(tcg_out, tcg_in);
-            break;
-        case 2:
-            tcg_gen_ext32u_i64(tcg_out, tcg_in);
-            break;
-        case 3:
-            tcg_gen_mov_i64(tcg_out, tcg_in);
-            break;
-        }
-    }
-
-    if (shift) {
-        tcg_gen_shli_i64(tcg_out, tcg_out, shift);
-    }
+    tcg_gen_ext_i64(tcg_out, tcg_in, extsize | (is_signed ? MO_SIGN : 0));
+    tcg_gen_shli_i64(tcg_out, tcg_out, shift);
 }
 
 static inline void gen_check_sp_alignment(DisasContext *s)
-- 
2.34.1

[PATCH 3/7] target/i386: Use tcg_gen_ext_tl

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/i386/tcg/translate.c | 28 +++-------------------------
 1 file changed, 3 insertions(+), 25 deletions(-)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 0c81e066de..d420ed8f0a 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -701,33 +701,11 @@ static inline void gen_op_movl_T0_Dshift(DisasContext *s, MemOp ot)
 
 static TCGv gen_ext_tl(TCGv dst, TCGv src, MemOp size, bool sign)
 {
-    switch (size) {
-    case MO_8:
-        if (sign) {
-            tcg_gen_ext8s_tl(dst, src);
-        } else {
-            tcg_gen_ext8u_tl(dst, src);
-        }
-        return dst;
-    case MO_16:
-        if (sign) {
-            tcg_gen_ext16s_tl(dst, src);
-        } else {
-            tcg_gen_ext16u_tl(dst, src);
-        }
-        return dst;
-#ifdef TARGET_X86_64
-    case MO_32:
-        if (sign) {
-            tcg_gen_ext32s_tl(dst, src);
-        } else {
-            tcg_gen_ext32u_tl(dst, src);
-        }
-        return dst;
-#endif
-    default:
+    if (memop_size(size) == TARGET_LONG_BITS) {
         return src;
     }
+    tcg_gen_ext_tl(dst, src, size | (sign ? MO_SIGN : 0));
+    return dst;
 }
 
 static void gen_extu(MemOp ot, TCGv reg)
-- 
2.34.1

[PATCH 4/7] target/m68k: Use tcg_gen_ext_i32

[Richard Henderson]

We still need to check OS_{BYTE,WORD,LONG},
because m68k includes floating point in OS_*.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/translate.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 4d0110de95..4a0b0b2703 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -520,21 +520,9 @@ static inline void gen_ext(TCGv res, TCGv val, int opsize, int sign)
 {
     switch (opsize) {
     case OS_BYTE:
-        if (sign) {
-            tcg_gen_ext8s_i32(res, val);
-        } else {
-            tcg_gen_ext8u_i32(res, val);
-        }
-        break;
     case OS_WORD:
-        if (sign) {
-            tcg_gen_ext16s_i32(res, val);
-        } else {
-            tcg_gen_ext16u_i32(res, val);
-        }
-        break;
     case OS_LONG:
-        tcg_gen_mov_i32(res, val);
+        tcg_gen_ext_i32(res, val, opsize | (sign ? MO_SIGN : 0));
         break;
     default:
         g_assert_not_reached();
@@ -1072,15 +1060,10 @@ static int gen_ea_mode_fp(CPUM68KState *env, DisasContext *s, int mode,
             tmp = tcg_temp_new();
             switch (opsize) {
             case OS_BYTE:
-                tcg_gen_ext8s_i32(tmp, reg);
-                gen_helper_exts32(tcg_env, fp, tmp);
-                break;
             case OS_WORD:
-                tcg_gen_ext16s_i32(tmp, reg);
-                gen_helper_exts32(tcg_env, fp, tmp);
-                break;
             case OS_LONG:
-                gen_helper_exts32(tcg_env, fp, reg);
+                tcg_gen_ext_i32(tmp, reg, opsize | MO_SIGN);
+                gen_helper_exts32(tcg_env, fp, tmp);
                 break;
             case OS_SINGLE:
                 gen_helper_extf32(tcg_env, fp, reg);
-- 
2.34.1

[PATCH 5/7] target/rx: Use tcg_gen_ext_i32

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/rx/translate.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/target/rx/translate.c b/target/rx/translate.c
index f8860830ae..c6ce717a95 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -492,13 +492,11 @@ static bool trans_MOV_ra(DisasContext *ctx, arg_MOV_ra *a)
 /* mov.<bwl> rs,rd */
 static bool trans_MOV_mm(DisasContext *ctx, arg_MOV_mm *a)
 {
-    static void (* const mov[])(TCGv ret, TCGv arg) = {
-        tcg_gen_ext8s_i32, tcg_gen_ext16s_i32, tcg_gen_mov_i32,
-    };
     TCGv tmp, mem, addr;
+
     if (a->lds == 3 && a->ldd == 3) {
         /* mov.<bwl> rs,rd */
-        mov[a->sz](cpu_regs[a->rd], cpu_regs[a->rs]);
+        tcg_gen_ext_i32(cpu_regs[a->rd], cpu_regs[a->rs], a->sz | MO_SIGN);
         return true;
     }
 
@@ -570,10 +568,7 @@ static bool trans_MOVU_mr(DisasContext *ctx, arg_MOVU_mr *a)
 /* movu.<bw> rs,rd */
 static bool trans_MOVU_rr(DisasContext *ctx, arg_MOVU_rr *a)
 {
-    static void (* const ext[])(TCGv ret, TCGv arg) = {
-        tcg_gen_ext8u_i32, tcg_gen_ext16u_i32,
-    };
-    ext[a->sz](cpu_regs[a->rd], cpu_regs[a->rs]);
+    tcg_gen_ext_i32(cpu_regs[a->rd], cpu_regs[a->rs], a->sz);
     return true;
 }
 
-- 
2.34.1

[PATCH 6/7] target/tricore: Use tcg_gen_*extract_tl

[Richard Henderson]

The EXTR instructions can use the extract opcodes.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/tricore/translate.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index dd812ec0f0..66553d1be0 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -6542,28 +6542,16 @@ static void decode_rrpw_extract_insert(DisasContext *ctx)
     switch (op2) {
     case OPC2_32_RRPW_EXTR:
         if (width == 0) {
-                tcg_gen_movi_tl(cpu_gpr_d[r3], 0);
-                break;
-        }
-
-        if (pos + width <= 32) {
-            /* optimize special cases */
-            if ((pos == 0) && (width == 8)) {
-                tcg_gen_ext8s_tl(cpu_gpr_d[r3], cpu_gpr_d[r1]);
-            } else if ((pos == 0) && (width == 16)) {
-                tcg_gen_ext16s_tl(cpu_gpr_d[r3], cpu_gpr_d[r1]);
-            } else {
-                tcg_gen_shli_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], 32 - pos - width);
-                tcg_gen_sari_tl(cpu_gpr_d[r3], cpu_gpr_d[r3], 32 - width);
-            }
+            tcg_gen_movi_tl(cpu_gpr_d[r3], 0);
+        } else if (pos + width <= 32) {
+            tcg_gen_sextract_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], pos, width);
         }
         break;
     case OPC2_32_RRPW_EXTR_U:
         if (width == 0) {
             tcg_gen_movi_tl(cpu_gpr_d[r3], 0);
         } else {
-            tcg_gen_shri_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], pos);
-            tcg_gen_andi_tl(cpu_gpr_d[r3], cpu_gpr_d[r3], ~0u >> (32-width));
+            tcg_gen_extract_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], pos, width);
         }
         break;
     case OPC2_32_RRPW_IMASK:
-- 
2.34.1

[PATCH 7/7] target/xtensa: Use tcg_gen_sextract_i32

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/xtensa/translate.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 54bee7ddba..de89940599 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -2262,17 +2262,7 @@ static void translate_salt(DisasContext *dc, const OpcodeArg arg[],
 static void translate_sext(DisasContext *dc, const OpcodeArg arg[],
                            const uint32_t par[])
 {
-    int shift = 31 - arg[2].imm;
-
-    if (shift == 24) {
-        tcg_gen_ext8s_i32(arg[0].out, arg[1].in);
-    } else if (shift == 16) {
-        tcg_gen_ext16s_i32(arg[0].out, arg[1].in);
-    } else {
-        TCGv_i32 tmp = tcg_temp_new_i32();
-        tcg_gen_shli_i32(tmp, arg[1].in, shift);
-        tcg_gen_sari_i32(arg[0].out, tmp, shift);
-    }
+    tcg_gen_sextract_i32(arg[0].out, arg[1].in, 0, arg[2].imm + 1);
 }
 
 static uint32_t test_exceptions_simcall(DisasContext *dc,
-- 
2.34.1

Re: [PATCH 1/7] tcg: Export tcg_gen_ext_{i32,i64,tl}

[Philippe Mathieu-Daudé]

On 19/10/23 20:29, Richard Henderson wrote:
> The two concrete type functions already existed, merely needing
> a bit of hardening to invalid inputs.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/tcg/tcg-op-common.h |  2 ++
>   include/tcg/tcg-op.h        |  2 ++
>   tcg/tcg-op-ldst.c           | 14 ++++++++++----
>   3 files changed, 14 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 4/7] target/m68k: Use tcg_gen_ext_i32

[Philippe Mathieu-Daudé]

On 19/10/23 20:29, Richard Henderson wrote:
> We still need to check OS_{BYTE,WORD,LONG},
> because m68k includes floating point in OS_*.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/translate.c | 23 +++--------------------
>   1 file changed, 3 insertions(+), 20 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 5/7] target/rx: Use tcg_gen_ext_i32

[Philippe Mathieu-Daudé]

On 19/10/23 20:29, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/rx/translate.c | 11 +++--------
>   1 file changed, 3 insertions(+), 8 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 3/7] target/i386: Use tcg_gen_ext_tl

[Philippe Mathieu-Daudé]

On 19/10/23 20:29, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/i386/tcg/translate.c | 28 +++-------------------------
>   1 file changed, 3 insertions(+), 25 deletions(-)
> 
> diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
> index 0c81e066de..d420ed8f0a 100644
> --- a/target/i386/tcg/translate.c
> +++ b/target/i386/tcg/translate.c
> @@ -701,33 +701,11 @@ static inline void gen_op_movl_T0_Dshift(DisasContext *s, MemOp ot)
>   
>   static TCGv gen_ext_tl(TCGv dst, TCGv src, MemOp size, bool sign)
>   {
> -    switch (size) {
> -    case MO_8:
> -        if (sign) {
> -            tcg_gen_ext8s_tl(dst, src);
> -        } else {
> -            tcg_gen_ext8u_tl(dst, src);
> -        }
> -        return dst;
> -    case MO_16:
> -        if (sign) {
> -            tcg_gen_ext16s_tl(dst, src);
> -        } else {
> -            tcg_gen_ext16u_tl(dst, src);
> -        }
> -        return dst;
> -#ifdef TARGET_X86_64
> -    case MO_32:
> -        if (sign) {
> -            tcg_gen_ext32s_tl(dst, src);
> -        } else {
> -            tcg_gen_ext32u_tl(dst, src);
> -        }
> -        return dst;
> -#endif
> -    default:
> +    if (memop_size(size) == TARGET_LONG_BITS) {
>           return src;
>       }
> +    tcg_gen_ext_tl(dst, src, size | (sign ? MO_SIGN : 0));
> +    return dst;
>   }

While here, I'd rename 'size' -> 'mop'. Regardless,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 6/7] target/tricore: Use tcg_gen_*extract_tl

[Bastian Koppelmann]

On Thu, Oct 19, 2023 at 11:29:20AM -0700, Richard Henderson wrote:
> The EXTR instructions can use the extract opcodes.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/tricore/translate.c | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)

Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

Cheers,
Bastian

Re: [PATCH 3/7] target/i386: Use tcg_gen_ext_tl

[Paolo Bonzini]

On 10/19/23 23:57, Philippe Mathieu-Daudé wrote:
> On 19/10/23 20:29, Richard Henderson wrote:
>> -    default:
>> +    if (memop_size(size) == TARGET_LONG_BITS) {
>>           return src;
>>       }

Any opinions about adding something like this on top?

------------------------- 8< -------------------------------
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH] include, target/i386: define and use MO_TL

This will also come in handy later for "less than" comparisons.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/include/exec/target_long.h b/include/exec/target_long.h
index 93c9472971f..3cd8e26a23f 100644
--- a/include/exec/target_long.h
+++ b/include/exec/target_long.h
@@ -29,12 +29,14 @@ typedef uint32_t target_ulong;
  #define TARGET_FMT_lx "%08x"
  #define TARGET_FMT_ld "%d"
  #define TARGET_FMT_lu "%u"
+#define MO_TL MO_32
  #elif TARGET_LONG_SIZE == 8
  typedef int64_t target_long;
  typedef uint64_t target_ulong;
  #define TARGET_FMT_lx "%016" PRIx64
  #define TARGET_FMT_ld "%" PRId64
  #define TARGET_FMT_lu "%" PRIu64
+#define MO_TL MO_64
  #else
  #error TARGET_LONG_SIZE undefined
  #endif
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index b0d62e83445..7bf7406dd8e 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -701,7 +701,7 @@ static inline void 
gen_op_movl_T0_Dshift(DisasContext *s, MemOp ot)

  static TCGv gen_ext_tl(TCGv dst, TCGv src, MemOp size, bool sign)
  {
-    if (memop_size(size) == TARGET_LONG_BITS) {
+    if (size == MO_TL) {
          return src;
      }
      tcg_gen_ext_tl(dst, src, size | (sign ? MO_SIGN : 0));
-----------------------------------------------------------

I can add it in my x86 series if desirable (I also have an
occurrence of memop_size(ot) < TARGET_LONG_BITS there, and it
can become just "ot < MO_TL").

>> +    tcg_gen_ext_tl(dst, src, size | (sign ? MO_SIGN : 0));
>> +    return dst;
>>   }
> 
> While here, I'd rename 'size' -> 'mop'. Regardless,

Not sure about that, because "size" should be just the low bits of MemOp 
(the MO_SIGN bit is passed separately).

Paolo

Re: [PATCH 5/7] target/rx: Use tcg_gen_ext_i32

[Yoshinori Sato]

On Fri, 20 Oct 2023 03:29:19 +0900,
Richard Henderson wrote:
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Yoshinori Sato <ysato@users.sourceforge.jp>

> ---
>  target/rx/translate.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/target/rx/translate.c b/target/rx/translate.c
> index f8860830ae..c6ce717a95 100644
> --- a/target/rx/translate.c
> +++ b/target/rx/translate.c
> @@ -492,13 +492,11 @@ static bool trans_MOV_ra(DisasContext *ctx, arg_MOV_ra *a)
>  /* mov.<bwl> rs,rd */
>  static bool trans_MOV_mm(DisasContext *ctx, arg_MOV_mm *a)
>  {
> -    static void (* const mov[])(TCGv ret, TCGv arg) = {
> -        tcg_gen_ext8s_i32, tcg_gen_ext16s_i32, tcg_gen_mov_i32,
> -    };
>      TCGv tmp, mem, addr;
> +
>      if (a->lds == 3 && a->ldd == 3) {
>          /* mov.<bwl> rs,rd */
> -        mov[a->sz](cpu_regs[a->rd], cpu_regs[a->rs]);
> +        tcg_gen_ext_i32(cpu_regs[a->rd], cpu_regs[a->rs], a->sz | MO_SIGN);
>          return true;
>      }
>  
> @@ -570,10 +568,7 @@ static bool trans_MOVU_mr(DisasContext *ctx, arg_MOVU_mr *a)
>  /* movu.<bw> rs,rd */
>  static bool trans_MOVU_rr(DisasContext *ctx, arg_MOVU_rr *a)
>  {
> -    static void (* const ext[])(TCGv ret, TCGv arg) = {
> -        tcg_gen_ext8u_i32, tcg_gen_ext16u_i32,
> -    };
> -    ext[a->sz](cpu_regs[a->rd], cpu_regs[a->rs]);
> +    tcg_gen_ext_i32(cpu_regs[a->rd], cpu_regs[a->rs], a->sz);
>      return true;
>  }
>  
> -- 
> 2.34.1
> 

-- 
Yosinori Sato

Re: [PATCH 3/7] target/i386: Use tcg_gen_ext_tl

[Richard Henderson]

On 10/21/23 00:59, Paolo Bonzini wrote:
> On 10/19/23 23:57, Philippe Mathieu-Daudé wrote:
>> On 19/10/23 20:29, Richard Henderson wrote:
>>> -    default:
>>> +    if (memop_size(size) == TARGET_LONG_BITS) {
>>>           return src;
>>>       }
> 
> Any opinions about adding something like this on top?
> 
> ------------------------- 8< -------------------------------
> From: Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH] include, target/i386: define and use MO_TL

Yes, that looks fine.

>   static TCGv gen_ext_tl(TCGv dst, TCGv src, MemOp size, bool sign)
>   {
> -    if (memop_size(size) == TARGET_LONG_BITS) {
> +    if (size == MO_TL) {

Yep.

> I can add it in my x86 series if desirable ...

That's probably fine; you may well get your PR in before my next.

>>> +    tcg_gen_ext_tl(dst, src, size | (sign ? MO_SIGN : 0));
>>> +    return dst;
>>>   }
>>
>> While here, I'd rename 'size' -> 'mop'. Regardless,
> 
> Not sure about that, because "size" should be just the low bits of MemOp (the MO_SIGN bit 
> is passed separately).

Agreed.


r~

Re: [PATCH 7/7] target/xtensa: Use tcg_gen_sextract_i32

[Max Filippov]

On Thu, Oct 19, 2023 at 11:29 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/xtensa/translate.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)

Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>

-- 
Thanks.
-- Max

Re: [PATCH 3/7] target/i386: Use tcg_gen_ext_tl

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 1134 bytes --]

Il dom 22 ott 2023, 03:29 Richard Henderson <richard.henderson@linaro.org>
ha scritto:

> >   static TCGv gen_ext_tl(TCGv dst, TCGv src, MemOp size, bool sign)
> >   {
> > -    if (memop_size(size) == TARGET_LONG_BITS) {
> > +    if (size == MO_TL) {
>
> Yep.
>
> > I can add it in my x86 series if desirable ...
>
> That's probably fine; you may well get your PR in before my next.
>

I will probably keep only SHA instructions for 8.2 (plus the VEX todos and
the reorganized checks) and delay the rest.

There are a bunch of things I would do in a slightly different manner now,
so it's better to clean up the generic x86 decoder code before implementing
the less orthogonal instruction formats from the one-byte. I should have
time to finish opcodes 0xC0 to 0xFF over the Christmas break in time for
9.0. :)

Paolo



> >>> +    tcg_gen_ext_tl(dst, src, size | (sign ? MO_SIGN : 0));
> >>> +    return dst;
> >>>   }
> >>
> >> While here, I'd rename 'size' -> 'mop'. Regardless,
> >
> > Not sure about that, because "size" should be just the low bits of MemOp
> (the MO_SIGN bit
> > is passed separately).
>
> Agreed.
>
>
> r~
>
>

[-- Attachment #2: Type: text/html, Size: 1978 bytes --]

Re: [PATCH 6/7] target/tricore: Use tcg_gen_*extract_tl

[Philippe Mathieu-Daudé]

On 19/10/23 20:29, Richard Henderson wrote:
> The EXTR instructions can use the extract opcodes.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/tricore/translate.c | 20 ++++----------------
>   1 file changed, 4 insertions(+), 16 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 2/7] target/arm: Use tcg_gen_ext_i64

[Philippe Mathieu-Daudé]

On 19/10/23 20:29, Richard Henderson wrote:
> The ext_and_shift_reg helper does this plus a shift.
> The non-zero check for shift count is duplicate to
> the one done within tcg_gen_shli_i64.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/arm/tcg/translate-a64.c | 37 ++--------------------------------
>   1 file changed, 2 insertions(+), 35 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 7/7] target/xtensa: Use tcg_gen_sextract_i32

[Philippe Mathieu-Daudé]

On 19/10/23 20:29, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/xtensa/translate.c | 12 +-----------
>   1 file changed, 1 insertion(+), 11 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>