[Qemu-devel] [PATCH 0/9] tcg: reorganize bswap* functions

[Qemu-devel] [PATCH 0/9] tcg: reorganize bswap* functions

[Aurelien Jarno]

Hi all,

This patch series tries to reorganize a bit the bswap* TCG functions.

Currently we have bswap16_i32, bswap_i32 and bswap_i64, which means that
on a 64-bit target operating on 16- or 32-bit data (eg lhbrw/lwbrx on
powerpc), we need first to convert the 64-bit register value to a 32-bit
value, then apply bswap16_i32 and convert it back to a 64-bit register.

The patches define bswap16 and bswap32 for both i32 and i64 types, as
well as bswap64 for i64 type. The corresponding instructions are
implemented for x86 and x86_64 backends.

Best regards,
Aurelien

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH 1/9] tcg: rename bswap_i32/i64 functions

[Aurelien Jarno]

Rename bswap_i32 into bswap32_i32 and bswap_i64 into bswap64_i64

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-arm/translate.c  |   12 ++++++------
 target-i386/translate.c |    6 +++---
 target-m68k/translate.c |    2 +-
 target-ppc/translate.c  |   22 +++++++++++-----------
 tcg/arm/tcg-target.h    |    2 +-
 tcg/hppa/tcg-target.h   |    2 +-
 tcg/i386/tcg-target.c   |    4 ++--
 tcg/i386/tcg-target.h   |    2 +-
 tcg/sparc/tcg-target.h  |    4 ++--
 tcg/tcg-op.h            |   18 +++++++++---------
 tcg/tcg-opc.h           |    8 ++++----
 tcg/x86_64/tcg-target.c |    8 ++++----
 tcg/x86_64/tcg-target.h |    4 ++--
 13 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 3cef021..7c3d472 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -5162,7 +5162,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
                         NEON_GET_REG(T0, rm, pass * 2);
                         NEON_GET_REG(T1, rm, pass * 2 + 1);
                         switch (size) {
-                        case 0: tcg_gen_bswap_i32(cpu_T[0], cpu_T[0]); break;
+                        case 0: tcg_gen_bswap32_i32(cpu_T[0], cpu_T[0]); break;
                         case 1: gen_swap_half(cpu_T[0]); break;
                         case 2: /* no-op */ break;
                         default: abort();
@@ -5173,7 +5173,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
                         } else {
                             gen_op_movl_T0_T1();
                             switch (size) {
-                            case 0: tcg_gen_bswap_i32(cpu_T[0], cpu_T[0]); break;
+                            case 0: tcg_gen_bswap32_i32(cpu_T[0], cpu_T[0]); break;
                             case 1: gen_swap_half(cpu_T[0]); break;
                             default: abort();
                             }
@@ -5315,7 +5315,7 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
                         switch (op) {
                         case 1: /* VREV32 */
                             switch (size) {
-                            case 0: tcg_gen_bswap_i32(cpu_T[0], cpu_T[0]); break;
+                            case 0: tcg_gen_bswap32_i32(cpu_T[0], cpu_T[0]); break;
                             case 1: gen_swap_half(cpu_T[0]); break;
                             default: return 1;
                             }
@@ -6568,7 +6568,7 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
                             if (insn & (1 << 7))
                                 gen_rev16(tmp);
                             else
-                                tcg_gen_bswap_i32(tmp, tmp);
+                                tcg_gen_bswap32_i32(tmp, tmp);
                         }
                         store_reg(s, rd, tmp);
                     } else {
@@ -7384,7 +7384,7 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                     gen_helper_rbit(tmp, tmp);
                     break;
                 case 0x08: /* rev */
-                    tcg_gen_bswap_i32(tmp, tmp);
+                    tcg_gen_bswap32_i32(tmp, tmp);
                     break;
                 case 0x09: /* rev16 */
                     gen_rev16(tmp);
@@ -8518,7 +8518,7 @@ static void disas_thumb_insn(CPUState *env, DisasContext *s)
             rd = insn & 0x7;
             tmp = load_reg(s, rn);
             switch ((insn >> 6) & 3) {
-            case 0: tcg_gen_bswap_i32(tmp, tmp); break;
+            case 0: tcg_gen_bswap32_i32(tmp, tmp); break;
             case 1: gen_rev16(tmp); break;
             case 3: gen_revsh(tmp); break;
             default: goto illegal_op;
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 4b894fd..6eb12ce 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -6640,7 +6640,7 @@ static target_ulong disas_insn(DisasContext *s, target_ulong pc_start)
 #ifdef TARGET_X86_64
         if (dflag == 2) {
             gen_op_mov_TN_reg(OT_QUAD, 0, reg);
-            tcg_gen_bswap_i64(cpu_T[0], cpu_T[0]);
+            tcg_gen_bswap64_i64(cpu_T[0], cpu_T[0]);
             gen_op_mov_reg_T0(OT_QUAD, reg);
         } else
         {
@@ -6649,14 +6649,14 @@ static target_ulong disas_insn(DisasContext *s, target_ulong pc_start)
             
             tmp0 = tcg_temp_new_i32();
             tcg_gen_trunc_i64_i32(tmp0, cpu_T[0]);
-            tcg_gen_bswap_i32(tmp0, tmp0);
+            tcg_gen_bswap32_i32(tmp0, tmp0);
             tcg_gen_extu_i32_i64(cpu_T[0], tmp0);
             gen_op_mov_reg_T0(OT_LONG, reg);
         }
 #else
         {
             gen_op_mov_TN_reg(OT_LONG, 0, reg);
-            tcg_gen_bswap_i32(cpu_T[0], cpu_T[0]);
+            tcg_gen_bswap32_i32(cpu_T[0], cpu_T[0]);
             gen_op_mov_reg_T0(OT_LONG, reg);
         }
 #endif
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 7a0d85d..4e3cf4a 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -1247,7 +1247,7 @@ DISAS_INSN(byterev)
     TCGv reg;
 
     reg = DREG(insn, 0);
-    tcg_gen_bswap_i32(reg, reg);
+    tcg_gen_bswap32_i32(reg, reg);
 }
 
 DISAS_INSN(move)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 0368c37..d30bd35 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2621,11 +2621,11 @@ static always_inline void gen_qemu_ld32u(DisasContext *ctx, TCGv arg1, TCGv arg2
 #if defined(TARGET_PPC64)
         TCGv_i32 t0 = tcg_temp_new_i32();
         tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_bswap_i32(t0, t0);
+        tcg_gen_bswap32_i32(t0, t0);
         tcg_gen_extu_i32_tl(arg1, t0);
         tcg_temp_free_i32(t0);
 #else
-        tcg_gen_bswap_i32(arg1, arg1);
+        tcg_gen_bswap32_i32(arg1, arg1);
 #endif
     }
 }
@@ -2638,7 +2638,7 @@ static always_inline void gen_qemu_ld32s(DisasContext *ctx, TCGv arg1, TCGv arg2
         tcg_gen_qemu_ld32u(arg1, arg2, ctx->mem_idx);
         t0 = tcg_temp_new_i32();
         tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_bswap_i32(t0, t0);
+        tcg_gen_bswap32_i32(t0, t0);
         tcg_gen_ext_i32_tl(arg1, t0);
         tcg_temp_free_i32(t0);
     } else
@@ -2650,7 +2650,7 @@ static always_inline void gen_qemu_ld64(DisasContext *ctx, TCGv_i64 arg1, TCGv a
 {
     tcg_gen_qemu_ld64(arg1, arg2, ctx->mem_idx);
     if (unlikely(ctx->le_mode)) {
-        tcg_gen_bswap_i64(arg1, arg1);
+        tcg_gen_bswap64_i64(arg1, arg1);
     }
 }
 
@@ -2694,7 +2694,7 @@ static always_inline void gen_qemu_st32(DisasContext *ctx, TCGv arg1, TCGv arg2)
         TCGv t1;
         t0 = tcg_temp_new_i32();
         tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_bswap_i32(t0, t0);
+        tcg_gen_bswap32_i32(t0, t0);
         t1 = tcg_temp_new();
         tcg_gen_extu_i32_tl(t1, t0);
         tcg_temp_free_i32(t0);
@@ -2702,7 +2702,7 @@ static always_inline void gen_qemu_st32(DisasContext *ctx, TCGv arg1, TCGv arg2)
         tcg_temp_free(t1);
 #else
         TCGv t0 = tcg_temp_new_i32();
-        tcg_gen_bswap_i32(t0, arg1);
+        tcg_gen_bswap32_i32(t0, arg1);
         tcg_gen_qemu_st32(t0, arg2, ctx->mem_idx);
         tcg_temp_free(t0);
 #endif
@@ -2715,7 +2715,7 @@ static always_inline void gen_qemu_st64(DisasContext *ctx, TCGv_i64 arg1, TCGv a
 {
     if (unlikely(ctx->le_mode)) {
         TCGv_i64 t0 = tcg_temp_new_i64();
-        tcg_gen_bswap_i64(t0, arg1);
+        tcg_gen_bswap64_i64(t0, arg1);
         tcg_gen_qemu_st64(t0, arg2, ctx->mem_idx);
         tcg_temp_free_i64(t0);
     } else
@@ -3014,11 +3014,11 @@ static void always_inline gen_qemu_ld32ur(DisasContext *ctx, TCGv arg1, TCGv arg
 #if defined(TARGET_PPC64)
         TCGv_i32 t0 = tcg_temp_new_i32();
         tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_bswap_i32(t0, t0);
+        tcg_gen_bswap32_i32(t0, t0);
         tcg_gen_extu_i32_tl(arg1, t0);
         tcg_temp_free_i32(t0);
 #else
-        tcg_gen_bswap_i32(arg1, arg1);
+        tcg_gen_bswap32_i32(arg1, arg1);
 #endif
     }
 }
@@ -3062,7 +3062,7 @@ static void always_inline gen_qemu_st32r(DisasContext *ctx, TCGv arg1, TCGv arg2
         TCGv t1;
         t0 = tcg_temp_new_i32();
         tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_bswap_i32(t0, t0);
+        tcg_gen_bswap32_i32(t0, t0);
         t1 = tcg_temp_new();
         tcg_gen_extu_i32_tl(t1, t0);
         tcg_temp_free_i32(t0);
@@ -3070,7 +3070,7 @@ static void always_inline gen_qemu_st32r(DisasContext *ctx, TCGv arg1, TCGv arg2
         tcg_temp_free(t1);
 #else
         TCGv t0 = tcg_temp_new_i32();
-        tcg_gen_bswap_i32(t0, arg1);
+        tcg_gen_bswap32_i32(t0, arg1);
         tcg_gen_qemu_st32(t0, arg2, ctx->mem_idx);
         tcg_temp_free(t0);
 #endif
diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
index cccb8f9..8f5016f 100644
--- a/tcg/arm/tcg-target.h
+++ b/tcg/arm/tcg-target.h
@@ -28,7 +28,7 @@
 #undef TCG_TARGET_WORDS_BIGENDIAN
 #undef TCG_TARGET_HAS_div_i32
 #undef TCG_TARGET_HAS_div_i64
-#undef TCG_TARGET_HAS_bswap_i32
+#undef TCG_TARGET_HAS_bswap32_i32
 #define TCG_TARGET_HAS_ext8s_i32
 #define TCG_TARGET_HAS_ext16s_i32
 #define TCG_TARGET_HAS_neg_i32
diff --git a/tcg/hppa/tcg-target.h b/tcg/hppa/tcg-target.h
index 02dd294..7ab6f0c 100644
--- a/tcg/hppa/tcg-target.h
+++ b/tcg/hppa/tcg-target.h
@@ -78,7 +78,7 @@ enum {
 //#define TCG_TARGET_HAS_ext8s_i32
 //#define TCG_TARGET_HAS_ext16s_i32
 //#define TCG_TARGET_HAS_bswap16_i32
-//#define TCG_TARGET_HAS_bswap_i32
+//#define TCG_TARGET_HAS_bswap32_i32
 
 /* Note: must be synced with dyngen-exec.h */
 #define TCG_AREG0 TCG_REG_R17
diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index c8f7fff..d8b5563 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -1034,7 +1034,7 @@ static inline void tcg_out_op(TCGContext *s, int opc,
         tcg_out_brcond2(s, args, const_args);
         break;
 
-    case INDEX_op_bswap_i32:
+    case INDEX_op_bswap32_i32:
         tcg_out_opc(s, (0xc8 + args[0]) | P_EXT);
         break;
 
@@ -1130,7 +1130,7 @@ static const TCGTargetOpDef x86_op_defs[] = {
     { INDEX_op_sub2_i32, { "r", "r", "0", "1", "ri", "ri" } },
     { INDEX_op_brcond2_i32, { "r", "r", "ri", "ri" } },
 
-    { INDEX_op_bswap_i32, { "r", "0" } },
+    { INDEX_op_bswap32_i32, { "r", "0" } },
 
     { INDEX_op_neg_i32, { "r", "0" } },
 
diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index b5dec57..60450a4 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -45,7 +45,7 @@ enum {
 #define TCG_TARGET_CALL_STACK_OFFSET 0
 
 /* optional instructions */
-#define TCG_TARGET_HAS_bswap_i32
+#define TCG_TARGET_HAS_bswap32_i32
 #define TCG_TARGET_HAS_neg_i32
 #define TCG_TARGET_HAS_not_i32
 #define TCG_TARGET_HAS_ext8s_i32
diff --git a/tcg/sparc/tcg-target.h b/tcg/sparc/tcg-target.h
index 22bd9fe..43ec818 100644
--- a/tcg/sparc/tcg-target.h
+++ b/tcg/sparc/tcg-target.h
@@ -86,8 +86,8 @@ enum {
 #endif
 
 /* optional instructions */
-//#define TCG_TARGET_HAS_bswap_i32
-//#define TCG_TARGET_HAS_bswap_i64
+//#define TCG_TARGET_HAS_bswap32_i32
+//#define TCG_TARGET_HAS_bswap64_i64
 //#define TCG_TARGET_HAS_neg_i32
 //#define TCG_TARGET_HAS_neg_i64
 
diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 190a365..fa0f01c 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -1220,10 +1220,10 @@ static inline void tcg_gen_bswap16_i32(TCGv_i32 ret, TCGv_i32 arg)
 #endif
 }
 
-static inline void tcg_gen_bswap_i32(TCGv_i32 ret, TCGv_i32 arg)
+static inline void tcg_gen_bswap32_i32(TCGv_i32 ret, TCGv_i32 arg)
 {
-#ifdef TCG_TARGET_HAS_bswap_i32
-    tcg_gen_op2_i32(INDEX_op_bswap_i32, ret, arg);
+#ifdef TCG_TARGET_HAS_bswap32_i32
+    tcg_gen_op2_i32(INDEX_op_bswap32_i32, ret, arg);
 #else
     TCGv_i32 t0, t1;
     t0 = tcg_temp_new_i32();
@@ -1300,14 +1300,14 @@ static inline void tcg_gen_ext_i32_i64(TCGv_i64 ret, TCGv_i32 arg)
     tcg_gen_sari_i32(TCGV_HIGH(ret), TCGV_LOW(ret), 31);
 }
 
-static inline void tcg_gen_bswap_i64(TCGv_i64 ret, TCGv_i64 arg)
+static inline void tcg_gen_bswap64_i64(TCGv_i64 ret, TCGv_i64 arg)
 {
     TCGv_i32 t0, t1;
     t0 = tcg_temp_new_i32();
     t1 = tcg_temp_new_i32();
 
-    tcg_gen_bswap_i32(t0, TCGV_LOW(arg));
-    tcg_gen_bswap_i32(t1, TCGV_HIGH(arg));
+    tcg_gen_bswap32_i32(t0, TCGV_LOW(arg));
+    tcg_gen_bswap32_i32(t1, TCGV_HIGH(arg));
     tcg_gen_mov_i32(TCGV_LOW(ret), t1);
     tcg_gen_mov_i32(TCGV_HIGH(ret), t0);
     tcg_temp_free_i32(t0);
@@ -1381,10 +1381,10 @@ static inline void tcg_gen_ext_i32_i64(TCGv_i64 ret, TCGv_i32 arg)
     tcg_gen_ext32s_i64(ret, MAKE_TCGV_I64(GET_TCGV_I32(arg)));
 }
 
-static inline void tcg_gen_bswap_i64(TCGv_i64 ret, TCGv_i64 arg)
+static inline void tcg_gen_bswap64_i64(TCGv_i64 ret, TCGv_i64 arg)
 {
-#ifdef TCG_TARGET_HAS_bswap_i64
-    tcg_gen_op2_i64(INDEX_op_bswap_i64, ret, arg);
+#ifdef TCG_TARGET_HAS_bswap64_i64
+    tcg_gen_op2_i64(INDEX_op_bswap64_i64, ret, arg);
 #else
     TCGv_i32 t0, t1;
     t0 = tcg_temp_new_i32();
diff --git a/tcg/tcg-opc.h b/tcg/tcg-opc.h
index 560c8a7..c7f2b80 100644
--- a/tcg/tcg-opc.h
+++ b/tcg/tcg-opc.h
@@ -89,8 +89,8 @@ DEF2(ext8s_i32, 1, 1, 0, 0)
 #ifdef TCG_TARGET_HAS_ext16s_i32
 DEF2(ext16s_i32, 1, 1, 0, 0)
 #endif
-#ifdef TCG_TARGET_HAS_bswap_i32
-DEF2(bswap_i32, 1, 1, 0, 0)
+#ifdef TCG_TARGET_HAS_bswap32_i32
+DEF2(bswap32_i32, 1, 1, 0, 0)
 #endif
 #ifdef TCG_TARGET_HAS_not_i32
 DEF2(not_i32, 1, 1, 0, 0)
@@ -149,8 +149,8 @@ DEF2(ext16s_i64, 1, 1, 0, 0)
 #ifdef TCG_TARGET_HAS_ext32s_i64
 DEF2(ext32s_i64, 1, 1, 0, 0)
 #endif
-#ifdef TCG_TARGET_HAS_bswap_i64
-DEF2(bswap_i64, 1, 1, 0, 0)
+#ifdef TCG_TARGET_HAS_bswap64_i64
+DEF2(bswap64_i64, 1, 1, 0, 0)
 #endif
 #ifdef TCG_TARGET_HAS_not_i64
 DEF2(not_i64, 1, 1, 0, 0)
diff --git a/tcg/x86_64/tcg-target.c b/tcg/x86_64/tcg-target.c
index a840ac6..b7c4aa6 100644
--- a/tcg/x86_64/tcg-target.c
+++ b/tcg/x86_64/tcg-target.c
@@ -1094,10 +1094,10 @@ static inline void tcg_out_op(TCGContext *s, int opc, const TCGArg *args,
                        args[3], P_REXW);
         break;
 
-    case INDEX_op_bswap_i32:
+    case INDEX_op_bswap32_i32:
         tcg_out_opc(s, (0xc8 + (args[0] & 7)) | P_EXT, 0, args[0], 0);
         break;
-    case INDEX_op_bswap_i64:
+    case INDEX_op_bswap64_i64:
         tcg_out_opc(s, (0xc8 + (args[0] & 7)) | P_EXT | P_REXW, 0, args[0], 0);
         break;
 
@@ -1287,8 +1287,8 @@ static const TCGTargetOpDef x86_64_op_defs[] = {
 
     { INDEX_op_brcond_i64, { "r", "re" } },
 
-    { INDEX_op_bswap_i32, { "r", "0" } },
-    { INDEX_op_bswap_i64, { "r", "0" } },
+    { INDEX_op_bswap32_i32, { "r", "0" } },
+    { INDEX_op_bswap64_i64, { "r", "0" } },
 
     { INDEX_op_neg_i32, { "r", "0" } },
     { INDEX_op_neg_i64, { "r", "0" } },
diff --git a/tcg/x86_64/tcg-target.h b/tcg/x86_64/tcg-target.h
index b7e7dec..79d22ab 100644
--- a/tcg/x86_64/tcg-target.h
+++ b/tcg/x86_64/tcg-target.h
@@ -56,8 +56,8 @@ enum {
 #define TCG_TARGET_CALL_STACK_OFFSET 0
 
 /* optional instructions */
-#define TCG_TARGET_HAS_bswap_i32
-#define TCG_TARGET_HAS_bswap_i64
+#define TCG_TARGET_HAS_bswap32_i32
+#define TCG_TARGET_HAS_bswap64_i64
 #define TCG_TARGET_HAS_neg_i32
 #define TCG_TARGET_HAS_neg_i64
 #define TCG_TARGET_HAS_not_i32
-- 
1.6.1.3

[Qemu-devel] [PATCH 2/9] tcg: allow bswap16_i32 to be implemented by TCG backends

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/tcg-opc.h |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tcg/tcg-opc.h b/tcg/tcg-opc.h
index c7f2b80..b5fc197 100644
--- a/tcg/tcg-opc.h
+++ b/tcg/tcg-opc.h
@@ -89,6 +89,9 @@ DEF2(ext8s_i32, 1, 1, 0, 0)
 #ifdef TCG_TARGET_HAS_ext16s_i32
 DEF2(ext16s_i32, 1, 1, 0, 0)
 #endif
+#ifdef TCG_TARGET_HAS_bswap16_i32
+DEF2(bswap16_i32, 1, 1, 0, 0)
+#endif
 #ifdef TCG_TARGET_HAS_bswap32_i32
 DEF2(bswap32_i32, 1, 1, 0, 0)
 #endif
-- 
1.6.1.3

[Qemu-devel] [PATCH 3/9] tcg: add bswap16_i64 and bswap32_i64 TCG ops

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/tcg-op.h  |   58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tcg/tcg-opc.h |    6 +++++
 2 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index fa0f01c..652a374 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -1300,6 +1300,18 @@ static inline void tcg_gen_ext_i32_i64(TCGv_i64 ret, TCGv_i32 arg)
     tcg_gen_sari_i32(TCGV_HIGH(ret), TCGV_LOW(ret), 31);
 }
 
+/* Note: we assume the six high bytes are set to zero */
+static inline void tcg_gen_bswap16_i64(TCGv_i64 ret, TCGv_i64 arg)
+{
+    tcg_gen_bswap16_i32(TCGV_LOW(ret), TCGV_LOW(arg));
+}
+
+/* Note: we assume the four high bytes are set to zero */
+static inline void tcg_gen_bswap32_i64(TCGv_i64 ret, TCGv_i64 arg)
+{
+    tcg_gen_bswap32_i32(TCGV_LOW(ret), TCGV_LOW(arg));
+}
+
 static inline void tcg_gen_bswap64_i64(TCGv_i64 ret, TCGv_i64 arg)
 {
     TCGv_i32 t0, t1;
@@ -1381,6 +1393,52 @@ static inline void tcg_gen_ext_i32_i64(TCGv_i64 ret, TCGv_i32 arg)
     tcg_gen_ext32s_i64(ret, MAKE_TCGV_I64(GET_TCGV_I32(arg)));
 }
 
+/* Note: we assume the six high bytes are set to zero */
+static inline void tcg_gen_bswap16_i64(TCGv_i64 ret, TCGv_i64 arg)
+{
+#ifdef TCG_TARGET_HAS_bswap16_i64
+    tcg_gen_op2_i64(INDEX_op_bswap16_i64, ret, arg);
+#else
+    TCGv_i64 t0, t1;
+    t0 = tcg_temp_new_i64();
+    t1 = tcg_temp_new_i64();
+
+    tcg_gen_shri_i64(t0, arg, 8);
+    tcg_gen_andi_i64(t1, arg, 0x000000ff);
+    tcg_gen_shli_i64(t1, t1, 8);
+    tcg_gen_or_i64(ret, t0, t1);
+    tcg_temp_free_i64(t0);
+    tcg_temp_free_i64(t1);
+#endif
+}
+
+/* Note: we assume the four high bytes are set to zero */
+static inline void tcg_gen_bswap32_i64(TCGv_i64 ret, TCGv_i64 arg)
+{
+#ifdef TCG_TARGET_HAS_bswap32_i64
+    tcg_gen_op2_i64(INDEX_op_bswap32_i64, ret, arg);
+#else
+    TCGv_i64 t0, t1;
+    t0 = tcg_temp_new_i64();
+    t1 = tcg_temp_new_i64();
+
+    tcg_gen_shli_i64(t0, arg, 24);
+
+    tcg_gen_andi_i64(t1, arg, 0x0000ff00);
+    tcg_gen_shli_i64(t1, t1, 8);
+    tcg_gen_or_i64(t0, t0, t1);
+
+    tcg_gen_shri_i64(t1, arg, 8);
+    tcg_gen_andi_i64(t1, t1, 0x0000ff00);
+    tcg_gen_or_i64(t0, t0, t1);
+
+    tcg_gen_shri_i64(t1, arg, 24);
+    tcg_gen_or_i64(ret, t0, t1);
+    tcg_temp_free_i64(t0);
+    tcg_temp_free_i64(t1);
+#endif
+}
+
 static inline void tcg_gen_bswap64_i64(TCGv_i64 ret, TCGv_i64 arg)
 {
 #ifdef TCG_TARGET_HAS_bswap64_i64
diff --git a/tcg/tcg-opc.h b/tcg/tcg-opc.h
index b5fc197..3a095fc 100644
--- a/tcg/tcg-opc.h
+++ b/tcg/tcg-opc.h
@@ -152,6 +152,12 @@ DEF2(ext16s_i64, 1, 1, 0, 0)
 #ifdef TCG_TARGET_HAS_ext32s_i64
 DEF2(ext32s_i64, 1, 1, 0, 0)
 #endif
+#ifdef TCG_TARGET_HAS_bswap16_i64
+DEF2(bswap16_i64, 1, 1, 0, 0)
+#endif
+#ifdef TCG_TARGET_HAS_bswap32_i64
+DEF2(bswap32_i64, 1, 1, 0, 0)
+#endif
 #ifdef TCG_TARGET_HAS_bswap64_i64
 DEF2(bswap64_i64, 1, 1, 0, 0)
 #endif
-- 
1.6.1.3

[Qemu-devel] [PATCH 4/9] tcg: add _tl aliases to bswap16/32/64 TCG ops

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/tcg-op.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 652a374..10bdd07 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -2068,6 +2068,9 @@ static inline void tcg_gen_qemu_st64(TCGv_i64 arg, TCGv addr, int mem_index)
 #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_bswap16_tl tcg_gen_bswap16_i64
+#define tcg_gen_bswap32_tl tcg_gen_bswap32_i64
+#define tcg_gen_bswap64_tl tcg_gen_bswap64_i64
 #define tcg_gen_concat_tl_i64 tcg_gen_concat32_i64
 #define tcg_gen_andc_tl tcg_gen_andc_i64
 #define tcg_gen_eqv_tl tcg_gen_eqv_i64
@@ -2131,6 +2134,8 @@ static inline void tcg_gen_qemu_st64(TCGv_i64 arg, TCGv addr, int mem_index)
 #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_bswap16_tl tcg_gen_bswap16_i32
+#define tcg_gen_bswap32_tl tcg_gen_bswap32_i32
 #define tcg_gen_concat_tl_i64 tcg_gen_concat_i32_i64
 #define tcg_gen_andc_tl tcg_gen_andc_i32
 #define tcg_gen_eqv_tl tcg_gen_eqv_i32
-- 
1.6.1.3

[Qemu-devel] [PATCH 6/9] target-ppc: use the new bswap* TCG ops

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-ppc/translate.c |  129 ++++++------------------------------------------
 1 files changed, 15 insertions(+), 114 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index d30bd35..952ee99 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2580,35 +2580,16 @@ static always_inline void gen_qemu_ld16u(DisasContext *ctx, TCGv arg1, TCGv arg2
 {
     tcg_gen_qemu_ld16u(arg1, arg2, ctx->mem_idx);
     if (unlikely(ctx->le_mode)) {
-#if defined(TARGET_PPC64)
-        TCGv_i32 t0 = tcg_temp_new_i32();
-        tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_bswap16_i32(t0, t0);
-        tcg_gen_extu_i32_tl(arg1, t0);
-        tcg_temp_free_i32(t0);
-#else
-        tcg_gen_bswap16_i32(arg1, arg1);
-#endif
+        tcg_gen_bswap16_tl(arg1, arg1);
     }
 }
 
 static always_inline void gen_qemu_ld16s(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
     if (unlikely(ctx->le_mode)) {
-#if defined(TARGET_PPC64)
-        TCGv_i32 t0;
         tcg_gen_qemu_ld16u(arg1, arg2, ctx->mem_idx);
-        t0 = tcg_temp_new_i32();
-        tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_bswap16_i32(t0, t0);
-        tcg_gen_extu_i32_tl(arg1, t0);
+        tcg_gen_bswap16_tl(arg1, arg1);
         tcg_gen_ext16s_tl(arg1, arg1);
-        tcg_temp_free_i32(t0);
-#else
-        tcg_gen_qemu_ld16u(arg1, arg2, ctx->mem_idx);
-        tcg_gen_bswap16_i32(arg1, arg1);
-        tcg_gen_ext16s_i32(arg1, arg1);
-#endif
     } else {
         tcg_gen_qemu_ld16s(arg1, arg2, ctx->mem_idx);
     }
@@ -2618,15 +2599,7 @@ static always_inline void gen_qemu_ld32u(DisasContext *ctx, TCGv arg1, TCGv arg2
 {
     tcg_gen_qemu_ld32u(arg1, arg2, ctx->mem_idx);
     if (unlikely(ctx->le_mode)) {
-#if defined(TARGET_PPC64)
-        TCGv_i32 t0 = tcg_temp_new_i32();
-        tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_bswap32_i32(t0, t0);
-        tcg_gen_extu_i32_tl(arg1, t0);
-        tcg_temp_free_i32(t0);
-#else
-        tcg_gen_bswap32_i32(arg1, arg1);
-#endif
+        tcg_gen_bswap32_tl(arg1, arg1);
     }
 }
 
@@ -2634,13 +2607,9 @@ static always_inline void gen_qemu_ld32u(DisasContext *ctx, TCGv arg1, TCGv arg2
 static always_inline void gen_qemu_ld32s(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
     if (unlikely(ctx->le_mode)) {
-        TCGv_i32 t0;
         tcg_gen_qemu_ld32u(arg1, arg2, ctx->mem_idx);
-        t0 = tcg_temp_new_i32();
-        tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_bswap32_i32(t0, t0);
-        tcg_gen_ext_i32_tl(arg1, t0);
-        tcg_temp_free_i32(t0);
+        tcg_gen_bswap32_tl(arg1, arg1);
+        tcg_gen_ext32s_tl(arg1, arg1);
     } else
         tcg_gen_qemu_ld32s(arg1, arg2, ctx->mem_idx);
 }
@@ -2662,25 +2631,11 @@ static always_inline void gen_qemu_st8(DisasContext *ctx, TCGv arg1, TCGv arg2)
 static always_inline void gen_qemu_st16(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
     if (unlikely(ctx->le_mode)) {
-#if defined(TARGET_PPC64)
-        TCGv_i32 t0;
-        TCGv t1;
-        t0 = tcg_temp_new_i32();
-        tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_ext16u_i32(t0, t0);
-        tcg_gen_bswap16_i32(t0, t0);
-        t1 = tcg_temp_new();
-        tcg_gen_extu_i32_tl(t1, t0);
-        tcg_temp_free_i32(t0);
-        tcg_gen_qemu_st16(t1, arg2, ctx->mem_idx);
-        tcg_temp_free(t1);
-#else
         TCGv t0 = tcg_temp_new();
         tcg_gen_ext16u_tl(t0, arg1);
-        tcg_gen_bswap16_i32(t0, t0);
+        tcg_gen_bswap16_tl(t0, t0);
         tcg_gen_qemu_st16(t0, arg2, ctx->mem_idx);
         tcg_temp_free(t0);
-#endif
     } else {
         tcg_gen_qemu_st16(arg1, arg2, ctx->mem_idx);
     }
@@ -2689,23 +2644,11 @@ static always_inline void gen_qemu_st16(DisasContext *ctx, TCGv arg1, TCGv arg2)
 static always_inline void gen_qemu_st32(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
     if (unlikely(ctx->le_mode)) {
-#if defined(TARGET_PPC64)
-        TCGv_i32 t0;
-        TCGv t1;
-        t0 = tcg_temp_new_i32();
-        tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_bswap32_i32(t0, t0);
-        t1 = tcg_temp_new();
-        tcg_gen_extu_i32_tl(t1, t0);
-        tcg_temp_free_i32(t0);
-        tcg_gen_qemu_st32(t1, arg2, ctx->mem_idx);
-        tcg_temp_free(t1);
-#else
-        TCGv t0 = tcg_temp_new_i32();
-        tcg_gen_bswap32_i32(t0, arg1);
+        TCGv t0 = tcg_temp_new();
+        tcg_gen_ext32u_tl(t0, arg1);
+        tcg_gen_bswap32_tl(t0, t0);
         tcg_gen_qemu_st32(t0, arg2, ctx->mem_idx);
         tcg_temp_free(t0);
-#endif
     } else {
         tcg_gen_qemu_st32(arg1, arg2, ctx->mem_idx);
     }
@@ -2993,15 +2936,7 @@ static void always_inline gen_qemu_ld16ur(DisasContext *ctx, TCGv arg1, TCGv arg
 {
     tcg_gen_qemu_ld16u(arg1, arg2, ctx->mem_idx);
     if (likely(!ctx->le_mode)) {
-#if defined(TARGET_PPC64)
-        TCGv_i32 t0 = tcg_temp_new_i32();
-        tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_bswap16_i32(t0, t0);
-        tcg_gen_extu_i32_tl(arg1, t0);
-        tcg_temp_free_i32(t0);
-#else
-        tcg_gen_bswap16_i32(arg1, arg1);
-#endif
+        tcg_gen_bswap16_tl(arg1, arg1);
     }
 }
 GEN_LDX(lhbr, ld16ur, 0x16, 0x18, PPC_INTEGER);
@@ -3011,15 +2946,7 @@ static void always_inline gen_qemu_ld32ur(DisasContext *ctx, TCGv arg1, TCGv arg
 {
     tcg_gen_qemu_ld32u(arg1, arg2, ctx->mem_idx);
     if (likely(!ctx->le_mode)) {
-#if defined(TARGET_PPC64)
-        TCGv_i32 t0 = tcg_temp_new_i32();
-        tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_bswap32_i32(t0, t0);
-        tcg_gen_extu_i32_tl(arg1, t0);
-        tcg_temp_free_i32(t0);
-#else
-        tcg_gen_bswap32_i32(arg1, arg1);
-#endif
+        tcg_gen_bswap32_tl(arg1, arg1);
     }
 }
 GEN_LDX(lwbr, ld32ur, 0x16, 0x10, PPC_INTEGER);
@@ -3028,25 +2955,11 @@ GEN_LDX(lwbr, ld32ur, 0x16, 0x10, PPC_INTEGER);
 static void always_inline gen_qemu_st16r(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
     if (likely(!ctx->le_mode)) {
-#if defined(TARGET_PPC64)
-        TCGv_i32 t0;
-        TCGv t1;
-        t0 = tcg_temp_new_i32();
-        tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_ext16u_i32(t0, t0);
-        tcg_gen_bswap16_i32(t0, t0);
-        t1 = tcg_temp_new();
-        tcg_gen_extu_i32_tl(t1, t0);
-        tcg_temp_free_i32(t0);
-        tcg_gen_qemu_st16(t1, arg2, ctx->mem_idx);
-        tcg_temp_free(t1);
-#else
         TCGv t0 = tcg_temp_new();
         tcg_gen_ext16u_tl(t0, arg1);
-        tcg_gen_bswap16_i32(t0, t0);
+        tcg_gen_bswap16_tl(t0, t0);
         tcg_gen_qemu_st16(t0, arg2, ctx->mem_idx);
         tcg_temp_free(t0);
-#endif
     } else {
         tcg_gen_qemu_st16(arg1, arg2, ctx->mem_idx);
     }
@@ -3057,23 +2970,11 @@ GEN_STX(sthbr, st16r, 0x16, 0x1C, PPC_INTEGER);
 static void always_inline gen_qemu_st32r(DisasContext *ctx, TCGv arg1, TCGv arg2)
 {
     if (likely(!ctx->le_mode)) {
-#if defined(TARGET_PPC64)
-        TCGv_i32 t0;
-        TCGv t1;
-        t0 = tcg_temp_new_i32();
-        tcg_gen_trunc_tl_i32(t0, arg1);
-        tcg_gen_bswap32_i32(t0, t0);
-        t1 = tcg_temp_new();
-        tcg_gen_extu_i32_tl(t1, t0);
-        tcg_temp_free_i32(t0);
-        tcg_gen_qemu_st32(t1, arg2, ctx->mem_idx);
-        tcg_temp_free(t1);
-#else
-        TCGv t0 = tcg_temp_new_i32();
-        tcg_gen_bswap32_i32(t0, arg1);
+        TCGv t0 = tcg_temp_new();
+        tcg_gen_ext32u_tl(t0, arg1);
+        tcg_gen_bswap32_tl(t0, t0);
         tcg_gen_qemu_st32(t0, arg2, ctx->mem_idx);
         tcg_temp_free(t0);
-#endif
     } else {
         tcg_gen_qemu_st32(arg1, arg2, ctx->mem_idx);
     }
-- 
1.6.1.3

[Qemu-devel] [PATCH 7/9] target-i386: use the new bswap* TCG ops

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target-i386/translate.c |   15 ++-------------
 1 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 6eb12ce..4bb8013 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -6643,23 +6643,12 @@ static target_ulong disas_insn(DisasContext *s, target_ulong pc_start)
             tcg_gen_bswap64_i64(cpu_T[0], cpu_T[0]);
             gen_op_mov_reg_T0(OT_QUAD, reg);
         } else
-        {
-            TCGv_i32 tmp0;
-            gen_op_mov_TN_reg(OT_LONG, 0, reg);
-            
-            tmp0 = tcg_temp_new_i32();
-            tcg_gen_trunc_i64_i32(tmp0, cpu_T[0]);
-            tcg_gen_bswap32_i32(tmp0, tmp0);
-            tcg_gen_extu_i32_i64(cpu_T[0], tmp0);
-            gen_op_mov_reg_T0(OT_LONG, reg);
-        }
-#else
+#endif
         {
             gen_op_mov_TN_reg(OT_LONG, 0, reg);
-            tcg_gen_bswap32_i32(cpu_T[0], cpu_T[0]);
+            tcg_gen_bswap32_tl(cpu_T[0], cpu_T[0]);
             gen_op_mov_reg_T0(OT_LONG, reg);
         }
-#endif
         break;
     case 0xd6: /* salc */
         if (CODE64(s))
-- 
1.6.1.3

[Qemu-devel] [PATCH 8/9] tcg/x86: add bswap16_i32 ops

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/i386/tcg-target.c |    6 ++++++
 tcg/i386/tcg-target.h |    1 +
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index d8b5563..e0fd434 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -1034,6 +1034,11 @@ static inline void tcg_out_op(TCGContext *s, int opc,
         tcg_out_brcond2(s, args, const_args);
         break;
 
+    case INDEX_op_bswap16_i32:
+        tcg_out8(s, 0x66);
+        tcg_out_modrm(s, 0xc1, SHIFT_ROL, args[0]);
+        tcg_out8(s, 8);
+        break;
     case INDEX_op_bswap32_i32:
         tcg_out_opc(s, (0xc8 + args[0]) | P_EXT);
         break;
@@ -1130,6 +1135,7 @@ static const TCGTargetOpDef x86_op_defs[] = {
     { INDEX_op_sub2_i32, { "r", "r", "0", "1", "ri", "ri" } },
     { INDEX_op_brcond2_i32, { "r", "r", "ri", "ri" } },
 
+    { INDEX_op_bswap16_i32, { "r", "0" } },
     { INDEX_op_bswap32_i32, { "r", "0" } },
 
     { INDEX_op_neg_i32, { "r", "0" } },
diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index 60450a4..301a5bf 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -45,6 +45,7 @@ enum {
 #define TCG_TARGET_CALL_STACK_OFFSET 0
 
 /* optional instructions */
+#define TCG_TARGET_HAS_bswap16_i32
 #define TCG_TARGET_HAS_bswap32_i32
 #define TCG_TARGET_HAS_neg_i32
 #define TCG_TARGET_HAS_not_i32
-- 
1.6.1.3

[Qemu-devel] [PATCH 9/9] tcg/x86_64: add bswap16_i{32, 64} and bswap32_i64 ops

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/x86_64/tcg-target.c |   10 ++++++++++
 tcg/x86_64/tcg-target.h |    3 +++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/tcg/x86_64/tcg-target.c b/tcg/x86_64/tcg-target.c
index b7c4aa6..92f0733 100644
--- a/tcg/x86_64/tcg-target.c
+++ b/tcg/x86_64/tcg-target.c
@@ -1094,7 +1094,14 @@ static inline void tcg_out_op(TCGContext *s, int opc, const TCGArg *args,
                        args[3], P_REXW);
         break;
 
+    case INDEX_op_bswap16_i32:
+    case INDEX_op_bswap16_i64:
+        tcg_out8(s, 0x66);
+        tcg_out_modrm(s, 0xc1, SHIFT_ROL, args[0]);
+        tcg_out8(s, 8);
+        break;
     case INDEX_op_bswap32_i32:
+    case INDEX_op_bswap32_i64:
         tcg_out_opc(s, (0xc8 + (args[0] & 7)) | P_EXT, 0, args[0], 0);
         break;
     case INDEX_op_bswap64_i64:
@@ -1287,7 +1294,10 @@ static const TCGTargetOpDef x86_64_op_defs[] = {
 
     { INDEX_op_brcond_i64, { "r", "re" } },
 
+    { INDEX_op_bswap16_i32, { "r", "0" } },
+    { INDEX_op_bswap16_i64, { "r", "0" } },
     { INDEX_op_bswap32_i32, { "r", "0" } },
+    { INDEX_op_bswap32_i64, { "r", "0" } },
     { INDEX_op_bswap64_i64, { "r", "0" } },
 
     { INDEX_op_neg_i32, { "r", "0" } },
diff --git a/tcg/x86_64/tcg-target.h b/tcg/x86_64/tcg-target.h
index 79d22ab..8cb05c6 100644
--- a/tcg/x86_64/tcg-target.h
+++ b/tcg/x86_64/tcg-target.h
@@ -56,7 +56,10 @@ enum {
 #define TCG_TARGET_CALL_STACK_OFFSET 0
 
 /* optional instructions */
+#define TCG_TARGET_HAS_bswap16_i32
+#define TCG_TARGET_HAS_bswap16_i64
 #define TCG_TARGET_HAS_bswap32_i32
+#define TCG_TARGET_HAS_bswap32_i64
 #define TCG_TARGET_HAS_bswap64_i64
 #define TCG_TARGET_HAS_neg_i32
 #define TCG_TARGET_HAS_neg_i64
-- 
1.6.1.3

[Qemu-devel] [PATCH 5/9] tcg: update README wrt recent bswap changes

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 tcg/README |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/tcg/README b/tcg/README
index b135509..b342802 100644
--- a/tcg/README
+++ b/tcg/README
@@ -263,16 +263,17 @@ ext32u_i64 t0, t1
 
 8, 16 or 32 bit sign/zero extension (both operands must have the same type)
 
-* bswap16_i32 t0, t1
+* bswap16_i32/i64 t0, t1
 
-16 bit byte swap on a 32 bit value. The two high order bytes must be set
-to zero.
+16 bit byte swap on a 32/64 bit value. The two/six high order bytes must be
+set to zero.
 
-* bswap_i32 t0, t1
+* bswap32_i32/i64 t0, t1
 
-32 bit byte swap
+32 bit byte swap on a 32/64 bit value. With a 32 bit value, the four high
+order bytes must be set to zero.
 
-* bswap_i64 t0, t1
+* bswap64_i64 t0, t1
 
 64 bit byte swap
 
-- 
1.6.1.3

Re: [Qemu-devel] [PATCH 0/9] tcg: reorganize bswap* functions

[Paul Brook]

> This patch series tries to reorganize a bit the bswap* TCG functions.

In principle this looks ok, however several implementation issues:

> +/* Note: we assume the six high bytes are set to zero */
> +static inline void tcg_gen_bswap16_i64(TCGv_i64 ret, TCGv_i64 arg)
> +{
> +    tcg_gen_bswap16_i32(TCGV_LOW(ret), TCGV_LOW(arg));
> +}
> +
> +/* Note: we assume the four high bytes are set to zero */
> +static inline void tcg_gen_bswap32_i64(TCGv_i64 ret, TCGv_i64 arg)
> +{
> +    tcg_gen_bswap32_i32(TCGV_LOW(ret), TCGV_LOW(arg));
> +}

I think we want to preserve the zero extension of the value, i.e. you want 
something along the lines of:

if (!TCGV_EQUAL_I64(ret, arg))
  tcg_gen_movi_i32(TCGV_HIGH(ret), 0);

- I'm not sure whether it's more efficient to do movi(ret, 0) or mov(ret, 
arg). With a bit of luck they'll both be optimized away most of the time.

> +/* Note: we assume the four high bytes are set to zero */
> +static inline void tcg_gen_bswap32_i64(TCGv_i64 ret, TCGv_i64 arg)
> +{
>...
> +    tcg_gen_shli_i64(t0, arg, 24);

Likewise this is missing a mask operation. ext32u_i64(t0, t0) is probably the 
most efficient way).

> +++ b/tcg/i386/tcg-target.c
> +    case INDEX_op_bswap16_i32:
> +        tcg_out8(s, 0x66);
> +        tcg_out_modrm(s, 0xc1, SHIFT_ROL, args[0]);
> +        tcg_out8(s, 8);

> +    { INDEX_op_bswap16_i32, { "r", "0" } },

This is wrong. The r/m field of the modrm byte can only address the first 4 
registers (AL-DL). Values 4-7 address te second bytes of these registers 
(a.k.a. AH-DH). I suspect you need to use the "q" constraint.

> +++ b/tcg/x86_64/tcg-target.c
> +    case INDEX_op_bswap16_i32:
> +    case INDEX_op_bswap16_i64:
> +        tcg_out8(s, 0x66);
> +        tcg_out_modrm(s, 0xc1, SHIFT_ROL, args[0]);
> +        tcg_out8(s, 8);
> +        break;

You need to use tcg_out_opc here to get REX prefixes. You need P_REXB to avoid 
the legacy encoding issues mentioned above, and the high bit of the r/m field 
also goes in the REX byte.

Paul

Re: [Qemu-devel] [PATCH 0/9] tcg: reorganize bswap* functions

[Aurelien Jarno]

On Thu, Mar 12, 2009 at 12:29:46PM +0000, Paul Brook wrote:
> > This patch series tries to reorganize a bit the bswap* TCG functions.
> 
> In principle this looks ok, however several implementation issues:

Thanks for the review.

> > +/* Note: we assume the six high bytes are set to zero */
> > +static inline void tcg_gen_bswap16_i64(TCGv_i64 ret, TCGv_i64 arg)
> > +{
> > +    tcg_gen_bswap16_i32(TCGV_LOW(ret), TCGV_LOW(arg));
> > +}
> > +
> > +/* Note: we assume the four high bytes are set to zero */
> > +static inline void tcg_gen_bswap32_i64(TCGv_i64 ret, TCGv_i64 arg)
> > +{
> > +    tcg_gen_bswap32_i32(TCGV_LOW(ret), TCGV_LOW(arg));
> > +}
> 
> I think we want to preserve the zero extension of the value, i.e. you want 
> something along the lines of:
> 
> if (!TCGV_EQUAL_I64(ret, arg))
>   tcg_gen_movi_i32(TCGV_HIGH(ret), 0);

That's why I have modified the documentation to say the high bytes have
to be zero, similar to the current bswap16_i32 implementation.

I think we want to keep this behavior as when operating on 16-bit or
32-bit values, the high bytes most often stay 0, so this operation
is useless.

I have checked on the PowerPC target that the high bytes are always 
zero when need (either explicitely, or as part as a load operation).

For the i386 target, this is something missing, IMHO I should update
the "target-i386: use the new bswap* TCG ops" instead and put the zero
extension there.

> - I'm not sure whether it's more efficient to do movi(ret, 0) or mov(ret, 
> arg). With a bit of luck they'll both be optimized away most of the time.

idem.

> > +/* Note: we assume the four high bytes are set to zero */
> > +static inline void tcg_gen_bswap32_i64(TCGv_i64 ret, TCGv_i64 arg)
> > +{
> >...
> > +    tcg_gen_shli_i64(t0, arg, 24);
> 
> Likewise this is missing a mask operation. ext32u_i64(t0, t0) is probably the 
> most efficient way).

idem.

> > +++ b/tcg/i386/tcg-target.c
> > +    case INDEX_op_bswap16_i32:
> > +        tcg_out8(s, 0x66);
> > +        tcg_out_modrm(s, 0xc1, SHIFT_ROL, args[0]);
> > +        tcg_out8(s, 8);
> 
> > +    { INDEX_op_bswap16_i32, { "r", "0" } },
> 
> This is wrong. The r/m field of the modrm byte can only address the first 4 
> registers (AL-DL). Values 4-7 address te second bytes of these registers 
> (a.k.a. AH-DH). I suspect you need to use the "q" constraint.

That's what I did first when using XCHG xH, xL. That's why I used ROLW
instead which is able to work on the 8 registers by accessing 16-bit
registers.

> > +++ b/tcg/x86_64/tcg-target.c
> > +    case INDEX_op_bswap16_i32:
> > +    case INDEX_op_bswap16_i64:
> > +        tcg_out8(s, 0x66);
> > +        tcg_out_modrm(s, 0xc1, SHIFT_ROL, args[0]);
> > +        tcg_out8(s, 8);
> > +        break;
> 
> You need to use tcg_out_opc here to get REX prefixes. You need P_REXB to avoid 
> the legacy encoding issues mentioned above, and the high bit of the r/m field 
> also goes in the REX byte.

Same here.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 0/9] tcg: reorganize bswap* functions

[Paul Brook]

> > I think we want to preserve the zero extension of the value, i.e. you
> > want something along the lines of:
> >
> > if (!TCGV_EQUAL_I64(ret, arg))
> >   tcg_gen_movi_i32(TCGV_HIGH(ret), 0);
>
> That's why I have modified the documentation to say the high bytes have
> to be zero, similar to the current bswap16_i32 implementation.

There are two issues here:

- Requiring a zero extended input value.
- Providing a zero extended result.

Your documentation covers the former requirement.  However it does not mention 
the latter.  The existing bswap16_i32 implementation does provide this 
guarantee (and can be simplified if we do not require this). I expect it is 
worth preserving the zero extension.

> > > +++ b/tcg/i386/tcg-target.c
> That's what I did first when using XCHG xH, xL. That's why I used ROLW
> instead which is able to work on the 8 registers by accessing 16-bit
> registers.

Ok. My mistake.

> > You need to use tcg_out_opc here to get REX prefixes. You need P_REXB to
> > avoid the legacy encoding issues mentioned above, and the high bit of the
> > r/m field also goes in the REX byte.
>
> Same here.

I'd managed to convince myself that tcg_out_modrm didn't use tcg_out_opc. 
Clearly I was wrong.


Paul

Re: [Qemu-devel] [PATCH 0/9] tcg: reorganize bswap* functions

[Aurelien Jarno]

On Thu, Mar 12, 2009 at 01:31:30PM +0000, Paul Brook wrote:
> > > I think we want to preserve the zero extension of the value, i.e. you
> > > want something along the lines of:
> > >
> > > if (!TCGV_EQUAL_I64(ret, arg))
> > >   tcg_gen_movi_i32(TCGV_HIGH(ret), 0);
> >
> > That's why I have modified the documentation to say the high bytes have
> > to be zero, similar to the current bswap16_i32 implementation.
> 
> There are two issues here:
> 
> - Requiring a zero extended input value.
> - Providing a zero extended result.
> 
> Your documentation covers the former requirement.  However it does not mention 
> the latter.  The existing bswap16_i32 implementation does provide this 
> guarantee (and can be simplified if we do not require this). I expect it is 
> worth preserving the zero extension.

Thanks for your explanations, I see your point. I'll fix that before
committing the series.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net