tcg-i386: Introduce specific deposit helpers

PATCH In review Jan Kiszka 4 participants 14 messages Sep 28, 2011 – Oct 1, 2011

Share & tools

Jan Kiszka

author

2011-09-28 12:28 UTC #0 lore raw

PATCH 1/1 tcg-i386: Introduce specific deposit helpers

x86 cannot provide an optimized generic deposit implementation. But at least for a few special cases, namely for writing bits 0..7, 8..15, and 0..15, a version using only a single instruction is feasible. Introducing such helpers improves emulating 16-bit x86 code on x86, but also rarer cases where 32-bit or 64-bit code accesses bytes or words.

Signed-off-by Jan Kiszka <[email protected]>

---
 tcg/i386/tcg-target.c |   25 +++++++++++++++++++++++++
 tcg/i386/tcg-target.h |    6 ++++++
 tcg/tcg-op.h          |   12 ++++++++++++
 tcg/tcg-opc.h         |    6 ++++++
 tcg/tcg.h             |    3 +++
 5 files changed, 52 insertions(+), 0 deletions(-)

tcg/i386/tcg-target.c +25 -0

index 281f87d..3f9d0ad 100644
@@ -1747,6 +1747,19 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc,
         break;
 #endif
 
+    OP_32_64(deposit8l):
+        tcg_out_modrm(s, OPC_MOVB_EvGv | P_REXB_R | P_REXB_RM,
+                      args[2], args[0]);
+        break;
+
+    OP_32_64(deposit8h):
+        tcg_out_modrm(s, OPC_MOVB_EvGv, args[2], args[0] + 4);
+        break;
+
+    OP_32_64(deposit16l):
+        tcg_out_modrm(s, OPC_MOVL_EvGv | P_DATA16, args[2], args[0]);
+        break;
+
     default:
         tcg_abort();
     }

@@ -1802,6 +1815,14 @@ static const TCGTargetOpDef x86_op_defs[] = {
 
     { INDEX_op_setcond_i32, { "q", "r", "ri" } },
 
+#if TCG_TARGET_REG_BITS == 64
+    { INDEX_op_deposit8l_i32, { "r", "0", "r" } },
+#else
+    { INDEX_op_deposit8l_i32, { "abcd", "0", "abcd" } },
+#endif
+    { INDEX_op_deposit8h_i32, { "abcd", "0", "abcd" } },
+    { INDEX_op_deposit16l_i32, { "r", "0", "r" } },
+
 #if TCG_TARGET_REG_BITS == 32
     { INDEX_op_mulu2_i32, { "a", "d", "a", "r" } },
     { INDEX_op_add2_i32, { "r", "r", "0", "1", "ri", "ri" } },

@@ -1853,6 +1874,10 @@ static const TCGTargetOpDef x86_op_defs[] = {
     { INDEX_op_ext8u_i64, { "r", "r" } },
     { INDEX_op_ext16u_i64, { "r", "r" } },
     { INDEX_op_ext32u_i64, { "r", "r" } },
+
+    { INDEX_op_deposit8l_i64, { "r", "0", "r" } },
+    { INDEX_op_deposit8h_i64, { "abcd", "0", "abcd" } },
+    { INDEX_op_deposit16l_i64, { "r", "0", "r" } },
 #endif
 
 #if TCG_TARGET_REG_BITS == 64

tcg/i386/tcg-target.h +6 -0

index 5088e47..54b2f60 100644
@@ -91,6 +91,9 @@ enum {
 #define TCG_TARGET_HAS_nand_i32         0
 #define TCG_TARGET_HAS_nor_i32          0
 #define TCG_TARGET_HAS_deposit_i32      0
+#define TCG_TARGET_HAS_deposit8l_i32    1
+#define TCG_TARGET_HAS_deposit8h_i32    1
+#define TCG_TARGET_HAS_deposit16l_i32   1
 
 #if TCG_TARGET_REG_BITS == 64
 #define TCG_TARGET_HAS_div2_i64         1

@@ -112,6 +115,9 @@ enum {
 #define TCG_TARGET_HAS_nand_i64         0
 #define TCG_TARGET_HAS_nor_i64          0
 #define TCG_TARGET_HAS_deposit_i64      0
+#define TCG_TARGET_HAS_deposit8l_i64    1
+#define TCG_TARGET_HAS_deposit8h_i64    1
+#define TCG_TARGET_HAS_deposit16l_i64   1
 #endif
 
 #define TCG_TARGET_HAS_GUEST_BASE

tcg/tcg-op.h +12 -0

index 404b637..80ffccb 100644
@@ -2047,6 +2047,12 @@ static inline void tcg_gen_deposit_i32(TCGv_i32 ret, TCGv_i32 arg1,
 {
     if (TCG_TARGET_HAS_deposit_i32) {
         tcg_gen_op5ii_i32(INDEX_op_deposit_i32, ret, arg1, arg2, ofs, len);
+    } else if (ofs == 0 && len == 8 && TCG_TARGET_HAS_deposit8l_i32) {
+        tcg_gen_op3_i32(INDEX_op_deposit8l_i32, ret, arg1, arg2);
+    } else if (ofs == 8 && len == 8 && TCG_TARGET_HAS_deposit8h_i32) {
+        tcg_gen_op3_i32(INDEX_op_deposit8h_i32, ret, arg1, arg2);
+    } else if (ofs == 0 && len == 16 && TCG_TARGET_HAS_deposit16l_i32) {
+        tcg_gen_op3_i32(INDEX_op_deposit16l_i32, ret, arg1, arg2);
     } else {
         uint32_t mask = (1u << len) - 1;
         TCGv_i32 t1 = tcg_temp_new_i32 ();

@@ -2066,6 +2072,12 @@ static inline void tcg_gen_deposit_i64(TCGv_i64 ret, TCGv_i64 arg1,
 {
     if (TCG_TARGET_HAS_deposit_i64) {
         tcg_gen_op5ii_i64(INDEX_op_deposit_i64, ret, arg1, arg2, ofs, len);
+    } else if (ofs == 0 && len == 8 && TCG_TARGET_HAS_deposit8l_i64) {
+        tcg_gen_op3_i32(INDEX_op_deposit8l_i64, ret, arg1, arg2);
+    } else if (ofs == 8 && len == 8 && TCG_TARGET_HAS_deposit8h_i64) {
+        tcg_gen_op3_i32(INDEX_op_deposit8h_i64, ret, arg1, arg2);
+    } else if (ofs == 0 && len == 16 && TCG_TARGET_HAS_deposit16l_i64) {
+        tcg_gen_op3_i32(INDEX_op_deposit16l_i64, ret, arg1, arg2);
     } else {
         uint64_t mask = (1ull << len) - 1;
         TCGv_i64 t1 = tcg_temp_new_i64 ();

tcg/tcg-opc.h +6 -0

index 8e06d03..0871d15 100644
@@ -80,6 +80,9 @@ DEF(sar_i32, 1, 2, 0, 0)
 DEF(rotl_i32, 1, 2, 0, IMPL(TCG_TARGET_HAS_rot_i32))
 DEF(rotr_i32, 1, 2, 0, IMPL(TCG_TARGET_HAS_rot_i32))
 DEF(deposit_i32, 1, 2, 2, IMPL(TCG_TARGET_HAS_deposit_i32))
+DEF(deposit8l_i32, 1, 2, 0, IMPL(TCG_TARGET_HAS_deposit8l_i32))
+DEF(deposit8h_i32, 1, 2, 0, IMPL(TCG_TARGET_HAS_deposit8h_i32))
+DEF(deposit16l_i32, 1, 2, 0, IMPL(TCG_TARGET_HAS_deposit16l_i32))
 
 DEF(brcond_i32, 0, 2, 2, TCG_OPF_BB_END | TCG_OPF_SIDE_EFFECTS)

@@ -139,6 +142,9 @@ DEF(sar_i64, 1, 2, 0, IMPL64)
 DEF(rotl_i64, 1, 2, 0, IMPL64 | IMPL(TCG_TARGET_HAS_rot_i64))
 DEF(rotr_i64, 1, 2, 0, IMPL64 | IMPL(TCG_TARGET_HAS_rot_i64))
 DEF(deposit_i64, 1, 2, 2, IMPL64 | IMPL(TCG_TARGET_HAS_deposit_i64))
+DEF(deposit8l_i64, 1, 2, 0, IMPL64 | IMPL(TCG_TARGET_HAS_deposit8l_i64))
+DEF(deposit8h_i64, 1, 2, 0, IMPL64 | IMPL(TCG_TARGET_HAS_deposit8h_i64))
+DEF(deposit16l_i64, 1, 2, 0, IMPL64 | IMPL(TCG_TARGET_HAS_deposit16l_i64))
 
 DEF(brcond_i64, 0, 2, 2, TCG_OPF_BB_END | TCG_OPF_SIDE_EFFECTS | IMPL64)
 DEF(ext8s_i64, 1, 1, 0, IMPL64 | IMPL(TCG_TARGET_HAS_ext8s_i64))

tcg/tcg.h +3 -0

index dc5e9c9..9a95d2b 100644
@@ -69,6 +69,9 @@ typedef uint64_t TCGRegSet;
 #define TCG_TARGET_HAS_nand_i64         0
 #define TCG_TARGET_HAS_nor_i64          0
 #define TCG_TARGET_HAS_deposit_i64      0
+#define TCG_TARGET_HAS_deposit8l_i64    0
+#define TCG_TARGET_HAS_deposit8h_i64    0
+#define TCG_TARGET_HAS_deposit16l_i64   0
 #endif
 
 /* Only one of DIV or DIV2 should be defined.  */

Signature · 1 line

1.7.3.4

Richard Henderson

2011-09-28 14:26 UTC #1 lore raw

tcg/i386/tcg-target.c static const TCGTargetOpDef x86_op_defs[] = { · line 1822

Quoted 5 lines · Jan Kiszka

tcg/i386/tcg-target.c +5 -0

+#if TCG_TARGET_REG_BITS == 64
+    { INDEX_op_deposit8l_i32, { "r", "0", "r" } },
+#else
+    { INDEX_op_deposit8l_i32, { "abcd", "0", "abcd" } },
+#endif

At minimum, abcd is spelled "q".

I don't particularly care for the 3 different opcodes. Perhaps we'd be better off with an inline predicate for when the deposit opcode is "valid"?

Jan Kiszka

author

2011-09-28 14:34 UTC #2 lore raw

Quoted 8 lines · Richard Henderson

tcg/i386/tcg-target.c static const TCGTargetOpDef x86_op_defs[] = { · line 1822
Quoted 5 lines · Jan Kiszka
tcg/i386/tcg-target.c +5 -0
+#if TCG_TARGET_REG_BITS == 64
+    { INDEX_op_deposit8l_i32, { "r", "0", "r" } },
+#else
+    { INDEX_op_deposit8l_i32, { "abcd", "0", "abcd" } },
+#endif
At minimum, abcd is spelled "q".

Ah, perfect.

Quoted 4 lines

I don't particularly care for the 3 different opcodes. Perhaps we'd be better off with an inline predicate for when the deposit opcode is "valid"?

We still need to dispatch at tcg generation time which variant is valid. Or what do you have in mind?

Thanks,
Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Richard Henderson

2011-09-28 15:19 UTC #3 lore raw

Quoted 5 lines · Jan Kiszka

Quoted 3 lines
I don't particularly care for the 3 different opcodes. Perhaps we'd be better off with an inline predicate for when the deposit opcode is "valid"?
We still need to dispatch at tcg generation time which variant is valid. Or what do you have in mind?

Yes, but we'd know at tcg generation time that it *does* fit one of the few patterns you point out.

The predicate would be used in e.g. tcg_gen_deposit_i32 to select either INDEX_op_deposit_i32 or the and/shift/and/or fallback.

Jan Kiszka

2011-09-28 22:07 UTC #4 lore raw

Quoted 5 lines · Richard Henderson

Quoted 4 lines · Jan Kiszka
Quoted 3 lines
I don't particularly care for the 3 different opcodes. Perhaps we'd be better off with an inline predicate for when the deposit opcode is "valid"?

Quoted 7 lines

Quoted 1 line
We still need to dispatch at tcg generation time which variant is valid. Or what do you have in mind?
Yes, but we'd know at tcg generation time that it *does* fit one of the few patterns you point out.
The predicate would be used in e.g. tcg_gen_deposit_i32 to select either INDEX_op_deposit_i32 or the and/shift/and/or fallback.

I think I'm slowly starting to understand:

#define TCG_TARGET_HAS_deposit_i32(ofs, len) \
    (((ofs) == 0 && (len) == 8 && TCG_TARGET_HAS_deposit8l_i32) || \
     ...)

defined by the i386 tcg target, right?

Jan

Richard Henderson

2011-09-28 22:47 UTC #5 lore raw

Quoted 14 lines · Jan Kiszka

Quoted 5 lines · Richard Henderson
Quoted 4 lines · Jan Kiszka
Quoted 3 lines
I don't particularly care for the 3 different opcodes. Perhaps we'd be better off with an inline predicate for when the deposit opcode is "valid"?
Quoted 7 lines
Quoted 1 line
We still need to dispatch at tcg generation time which variant is valid. Or what do you have in mind?
Yes, but we'd know at tcg generation time that it *does* fit one of the few patterns you point out.
The predicate would be used in e.g. tcg_gen_deposit_i32 to select either INDEX_op_deposit_i32 or the and/shift/and/or fallback.

Quoted 7 lines

I think I'm slowly starting to understand:
#define TCG_TARGET_HAS_deposit_i32(ofs, len) \
    (((ofs) == 0 && (len) == 8 && TCG_TARGET_HAS_deposit8l_i32) || \
     ...)
defined by the i386 tcg target, right?

Something like that. Except without the TCG_TARGET_HAS_deposit8l_i32.

Jan Kiszka

author

2011-09-29 11:31 UTC #6 lore raw

PATCH v2 1/1 tcg-i386: Introduce limited deposit support

x86 cannot provide an optimized generic deposit implementation. But at least for a few special cases, namely for writing bits 0..7, 8..15, and 0..15, versions using only a single instruction are feasible. Introducing such limited support improves emulating 16-bit x86 code on x86, but also rarer cases where 32-bit or 64-bit code accesses bytes or words.

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v2:
- introduce restricting predicates TCG_TARGET_deposit_i32/64_valid to decide if deposit support can be used
- express register constraints via new 'Q' symbol

 tcg/i386/tcg-target.c |   24 ++++++++++++++++++++++++
 tcg/i386/tcg-target.h |    9 +++++++--
 tcg/tcg-op.h          |    4 ++--
 3 files changed, 33 insertions(+), 4 deletions(-)

tcg/i386/tcg-target.c +24 -0

index 281f87d..3069e53 100644
@@ -168,6 +168,10 @@ static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
             tcg_regset_set32(ct->u.regs, 0, 0xf);
         }
         break;
+    case 'Q':
+        ct->ct |= TCG_CT_REG;
+        tcg_regset_set32(ct->u.regs, 0, 0xf);
+        break;
     case 'r':
         ct->ct |= TCG_CT_REG;
         if (TCG_TARGET_REG_BITS == 64) {

@@ -1747,6 +1751,22 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc,
         break;
 #endif
 
+    OP_32_64(deposit):
+        if (args[3] == 0 && args[4] == 8) {
+            /* load bits 0..7 */
+            tcg_out_modrm(s, OPC_MOVB_EvGv | P_REXB_R | P_REXB_RM,
+                          args[2], args[0]);
+        } else if (args[3] == 8 && args[4] == 8) {
+            /* load bits 8..15 */
+            tcg_out_modrm(s, OPC_MOVB_EvGv, args[2], args[0] + 4);
+        } else if (args[3] == 0 && args[4] == 16) {
+            /* load bits 0..15 */
+            tcg_out_modrm(s, OPC_MOVL_EvGv | P_DATA16, args[2], args[0]);
+        } else {
+            tcg_abort();
+        }
+        break;
+
     default:
         tcg_abort();
     }

@@ -1802,6 +1822,8 @@ static const TCGTargetOpDef x86_op_defs[] = {
 
     { INDEX_op_setcond_i32, { "q", "r", "ri" } },
 
+    { INDEX_op_deposit_i32, { "Q", "0", "Q" } },
+
 #if TCG_TARGET_REG_BITS == 32
     { INDEX_op_mulu2_i32, { "a", "d", "a", "r" } },
     { INDEX_op_add2_i32, { "r", "r", "0", "1", "ri", "ri" } },

@@ -1853,6 +1875,8 @@ static const TCGTargetOpDef x86_op_defs[] = {
     { INDEX_op_ext8u_i64, { "r", "r" } },
     { INDEX_op_ext16u_i64, { "r", "r" } },
     { INDEX_op_ext32u_i64, { "r", "r" } },
+
+    { INDEX_op_deposit_i64, { "Q", "0", "Q" } },
 #endif
 
 #if TCG_TARGET_REG_BITS == 64

tcg/i386/tcg-target.h +7 -2

index 5088e47..b9c9d4e 100644
@@ -90,7 +90,7 @@ enum {
 #define TCG_TARGET_HAS_eqv_i32          0
 #define TCG_TARGET_HAS_nand_i32         0
 #define TCG_TARGET_HAS_nor_i32          0
-#define TCG_TARGET_HAS_deposit_i32      0
+#define TCG_TARGET_HAS_deposit_i32      1
 
 #if TCG_TARGET_REG_BITS == 64
 #define TCG_TARGET_HAS_div2_i64         1

@@ -111,9 +111,14 @@ enum {
 #define TCG_TARGET_HAS_eqv_i64          0
 #define TCG_TARGET_HAS_nand_i64         0
 #define TCG_TARGET_HAS_nor_i64          0
-#define TCG_TARGET_HAS_deposit_i64      0
+#define TCG_TARGET_HAS_deposit_i64      1
 #endif
 
+#define TCG_TARGET_deposit_i32_valid(ofs, len) \
+    (((ofs) == 0 && (len) == 8) || ((ofs) == 8 && (len) == 8) || \
+     ((ofs) == 0 && (len) == 16))
+#define TCG_TARGET_deposit_i64_valid    TCG_TARGET_deposit_i32_valid
+
 #define TCG_TARGET_HAS_GUEST_BASE
 
 /* Note: must be synced with dyngen-exec.h */

tcg/tcg-op.h +2 -2

index 404b637..fea5983 100644
@@ -2045,7 +2045,7 @@ static inline void tcg_gen_deposit_i32(TCGv_i32 ret, TCGv_i32 arg1,
 				       TCGv_i32 arg2, unsigned int ofs,
 				       unsigned int len)
 {
-    if (TCG_TARGET_HAS_deposit_i32) {
+    if (TCG_TARGET_HAS_deposit_i32 && TCG_TARGET_deposit_i32_valid(ofs, len)) {
         tcg_gen_op5ii_i32(INDEX_op_deposit_i32, ret, arg1, arg2, ofs, len);
     } else {
         uint32_t mask = (1u << len) - 1;

@@ -2064,7 +2064,7 @@ static inline void tcg_gen_deposit_i64(TCGv_i64 ret, TCGv_i64 arg1,
 				       TCGv_i64 arg2, unsigned int ofs,
 				       unsigned int len)
 {
-    if (TCG_TARGET_HAS_deposit_i64) {
+    if (TCG_TARGET_HAS_deposit_i64 && TCG_TARGET_deposit_i64_valid(ofs, len)) {
         tcg_gen_op5ii_i64(INDEX_op_deposit_i64, ret, arg1, arg2, ofs, len);
     } else {
         uint64_t mask = (1ull << len) - 1;

Signature · 1 line

1.7.3.4

Richard Henderson

2011-09-29 14:58 UTC #7 lore raw

Re: [Qemu-devel] [PATCH v2] tcg-i386: Introduce limited deposit support

Quoted 7 lines · Jan Kiszka

x86 cannot provide an optimized generic deposit implementation. But at least for a few special cases, namely for writing bits 0..7, 8..15, and 0..15, versions using only a single instruction are feasible. Introducing such limited support improves emulating 16-bit x86 code on x86, but also rarer cases where 32-bit or 64-bit code accesses bytes or words.

Quoted 7 lines

Signed-off-by Jan Kiszka <[email protected]>
---
Changes in v2:
- introduce restricting predicates TCG_TARGET_deposit_i32/64_valid to decide if deposit support can be used
- express register constraints via new 'Q' symbol

That's what I had in mind, yes.

The only thing that's missing now is a default version of

tcg/i386/tcg-target.h enum { · line 120

Quoted 4 lines

tcg/i386/tcg-target.h +4 -0

+#define TCG_TARGET_deposit_i32_valid(ofs, len) \
+    (((ofs) == 0 && (len) == 8) || ((ofs) == 8 && (len) == 8) || \
+     ((ofs) == 0 && (len) == 16))
+#define TCG_TARGET_deposit_i64_valid    TCG_TARGET_deposit_i32_valid

These.

Jan Kiszka

author

2011-09-29 15:20 UTC #8 lore raw

Re: [Qemu-devel] [PATCH v2] tcg-i386: Introduce limited deposit support

Quoted 18 lines · Richard Henderson

Quoted 7 lines · Jan Kiszka
x86 cannot provide an optimized generic deposit implementation. But at least for a few special cases, namely for writing bits 0..7, 8..15, and 0..15, versions using only a single instruction are feasible. Introducing such limited support improves emulating 16-bit x86 code on x86, but also rarer cases where 32-bit or 64-bit code accesses bytes or words.
Quoted 7 lines
Signed-off-by Jan Kiszka <[email protected]>
---
Changes in v2:
- introduce restricting predicates TCG_TARGET_deposit_i32/64_valid to decide if deposit support can be used
- express register constraints via new 'Q' symbol
That's what I had in mind, yes.

Quoted 8 lines

The only thing that's missing now is a default version of
tcg/i386/tcg-target.h enum { · line 120
Quoted 4 lines
tcg/i386/tcg-target.h +4 -0
+#define TCG_TARGET_deposit_i32_valid(ofs, len) \
+    (((ofs) == 0 && (len) == 8) || ((ofs) == 8 && (len) == 8) || \
+     ((ofs) == 0 && (len) == 16))
+#define TCG_TARGET_deposit_i64_valid    TCG_TARGET_deposit_i32_valid
These.

Interestingly, that didn't break the build here. Seems like gcc became too smart to report this.

Jan

Signature · 2 lines

Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Jan Kiszka

author

2011-09-29 15:23 UTC #9 lore raw

PATCH v3 1/1 tcg-i386: Introduce limited deposit support

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v3:
- provide default TCG_TARGET_deposit_i32_valid - just in case

Changes in v2:
- introduce restricting predicates TCG_TARGET_deposit_i32/64_valid to decide if deposit support can be used
- express register constraints via new 'Q' symbol

 tcg/i386/tcg-target.c |   24 ++++++++++++++++++++++++
 tcg/i386/tcg-target.h |    9 +++++++--
 tcg/tcg-op.h          |    4 ++--
 tcg/tcg.h             |    7 +++++++
 4 files changed, 40 insertions(+), 4 deletions(-)

tcg/i386/tcg-target.c +24 -0

index 281f87d..3069e53 100644
@@ -168,6 +168,10 @@ static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
             tcg_regset_set32(ct->u.regs, 0, 0xf);
         }
         break;
+    case 'Q':
+        ct->ct |= TCG_CT_REG;
+        tcg_regset_set32(ct->u.regs, 0, 0xf);
+        break;
     case 'r':
         ct->ct |= TCG_CT_REG;
         if (TCG_TARGET_REG_BITS == 64) {

@@ -1747,6 +1751,22 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc,
         break;
 #endif
 
+    OP_32_64(deposit):
+        if (args[3] == 0 && args[4] == 8) {
+            /* load bits 0..7 */
+            tcg_out_modrm(s, OPC_MOVB_EvGv | P_REXB_R | P_REXB_RM,
+                          args[2], args[0]);
+        } else if (args[3] == 8 && args[4] == 8) {
+            /* load bits 8..15 */
+            tcg_out_modrm(s, OPC_MOVB_EvGv, args[2], args[0] + 4);
+        } else if (args[3] == 0 && args[4] == 16) {
+            /* load bits 0..15 */
+            tcg_out_modrm(s, OPC_MOVL_EvGv | P_DATA16, args[2], args[0]);
+        } else {
+            tcg_abort();
+        }
+        break;
+
     default:
         tcg_abort();
     }

@@ -1802,6 +1822,8 @@ static const TCGTargetOpDef x86_op_defs[] = {
 
     { INDEX_op_setcond_i32, { "q", "r", "ri" } },
 
+    { INDEX_op_deposit_i32, { "Q", "0", "Q" } },
+
 #if TCG_TARGET_REG_BITS == 32
     { INDEX_op_mulu2_i32, { "a", "d", "a", "r" } },
     { INDEX_op_add2_i32, { "r", "r", "0", "1", "ri", "ri" } },

@@ -1853,6 +1875,8 @@ static const TCGTargetOpDef x86_op_defs[] = {
     { INDEX_op_ext8u_i64, { "r", "r" } },
     { INDEX_op_ext16u_i64, { "r", "r" } },
     { INDEX_op_ext32u_i64, { "r", "r" } },
+
+    { INDEX_op_deposit_i64, { "Q", "0", "Q" } },
 #endif
 
 #if TCG_TARGET_REG_BITS == 64

tcg/i386/tcg-target.h +7 -2

index 5088e47..b9c9d4e 100644
@@ -90,7 +90,7 @@ enum {
 #define TCG_TARGET_HAS_eqv_i32          0
 #define TCG_TARGET_HAS_nand_i32         0
 #define TCG_TARGET_HAS_nor_i32          0
-#define TCG_TARGET_HAS_deposit_i32      0
+#define TCG_TARGET_HAS_deposit_i32      1
 
 #if TCG_TARGET_REG_BITS == 64
 #define TCG_TARGET_HAS_div2_i64         1

@@ -111,9 +111,14 @@ enum {
 #define TCG_TARGET_HAS_eqv_i64          0
 #define TCG_TARGET_HAS_nand_i64         0
 #define TCG_TARGET_HAS_nor_i64          0
-#define TCG_TARGET_HAS_deposit_i64      0
+#define TCG_TARGET_HAS_deposit_i64      1
 #endif
 
+#define TCG_TARGET_deposit_i32_valid(ofs, len) \
+    (((ofs) == 0 && (len) == 8) || ((ofs) == 8 && (len) == 8) || \
+     ((ofs) == 0 && (len) == 16))
+#define TCG_TARGET_deposit_i64_valid    TCG_TARGET_deposit_i32_valid
+
 #define TCG_TARGET_HAS_GUEST_BASE
 
 /* Note: must be synced with dyngen-exec.h */

tcg/tcg-op.h +2 -2

index 404b637..fea5983 100644
@@ -2045,7 +2045,7 @@ static inline void tcg_gen_deposit_i32(TCGv_i32 ret, TCGv_i32 arg1,
 				       TCGv_i32 arg2, unsigned int ofs,
 				       unsigned int len)
 {
-    if (TCG_TARGET_HAS_deposit_i32) {
+    if (TCG_TARGET_HAS_deposit_i32 && TCG_TARGET_deposit_i32_valid(ofs, len)) {
         tcg_gen_op5ii_i32(INDEX_op_deposit_i32, ret, arg1, arg2, ofs, len);
     } else {
         uint32_t mask = (1u << len) - 1;

@@ -2064,7 +2064,7 @@ static inline void tcg_gen_deposit_i64(TCGv_i64 ret, TCGv_i64 arg1,
 				       TCGv_i64 arg2, unsigned int ofs,
 				       unsigned int len)
 {
-    if (TCG_TARGET_HAS_deposit_i64) {
+    if (TCG_TARGET_HAS_deposit_i64 && TCG_TARGET_deposit_i64_valid(ofs, len)) {
         tcg_gen_op5ii_i64(INDEX_op_deposit_i64, ret, arg1, arg2, ofs, len);
     } else {
         uint64_t mask = (1ull << len) - 1;

tcg/tcg.h +7 -0

index dc5e9c9..520255a 100644
@@ -71,6 +71,13 @@ typedef uint64_t TCGRegSet;
 #define TCG_TARGET_HAS_deposit_i64      0
 #endif
 
+#ifndef TCG_TARGET_deposit_i32_valid
+#define TCG_TARGET_deposit_i32_valid(ofs, len) 0
+#endif
+#ifndef TCG_TARGET_deposit_i64_valid
+#define TCG_TARGET_deposit_i64_valid(ofs, len) 0
+#endif
+
 /* Only one of DIV or DIV2 should be defined.  */
 #if defined(TCG_TARGET_HAS_div_i32)
 #define TCG_TARGET_HAS_div2_i32         0

Signature · 1 line

1.7.3.4

Richard Henderson

2011-09-29 16:11 UTC #10 lore raw

Re: [Qemu-devel] [PATCH v3] tcg-i386: Introduce limited deposit support

tcg/tcg.h typedef uint64_t TCGRegSet; · line 79

Quoted 6 lines · Jan Kiszka

tcg/tcg.h +6 -0

+#ifndef TCG_TARGET_deposit_i32_valid
+#define TCG_TARGET_deposit_i32_valid(ofs, len) 0
+#endif
+#ifndef TCG_TARGET_deposit_i64_valid
+#define TCG_TARGET_deposit_i64_valid(ofs, len) 0
+#endif

Err, no. The default is true. The targets that currently implement deposit at present can handle arbitrary inputs.

Jan Kiszka

author

2011-09-29 16:52 UTC #11 lore raw

PATCH v4 1/1 tcg-i386: Introduce limited deposit support

Quoted 10 lines · Richard Henderson

Review
Quoted 6 lines · Jan Kiszka
(quoted diff) +6 -0
+#ifndef TCG_TARGET_deposit_i32_valid
+#define TCG_TARGET_deposit_i32_valid(ofs, len) 0
+#endif
+#ifndef TCG_TARGET_deposit_i64_valid
+#define TCG_TARGET_deposit_i64_valid(ofs, len) 0
+#endif
Err, no. The default is true. The targets that currently implement deposit at present can handle arbitrary inputs.

Grr, of course.

---

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v4:
- provide correct default TCG_TARGET_deposit_i32_valid

Changes in v3:
- provide default TCG_TARGET_deposit_i32_valid - just in case

Changes in v2:
- introduce restricting predicates TCG_TARGET_deposit_i32/64_valid to decide if deposit support can be used
- express register constraints via new 'Q' symbol

 tcg/i386/tcg-target.c |   24 ++++++++++++++++++++++++
 tcg/i386/tcg-target.h |    9 +++++++--
 tcg/tcg-op.h          |    4 ++--
 tcg/tcg.h             |    7 +++++++
 4 files changed, 40 insertions(+), 4 deletions(-)

tcg/i386/tcg-target.c +24 -0

index 281f87d..3069e53 100644
@@ -168,6 +168,10 @@ static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
             tcg_regset_set32(ct->u.regs, 0, 0xf);
         }
         break;
+    case 'Q':
+        ct->ct |= TCG_CT_REG;
+        tcg_regset_set32(ct->u.regs, 0, 0xf);
+        break;
     case 'r':
         ct->ct |= TCG_CT_REG;
         if (TCG_TARGET_REG_BITS == 64) {

@@ -1747,6 +1751,22 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc,
         break;
 #endif
 
+    OP_32_64(deposit):
+        if (args[3] == 0 && args[4] == 8) {
+            /* load bits 0..7 */
+            tcg_out_modrm(s, OPC_MOVB_EvGv | P_REXB_R | P_REXB_RM,
+                          args[2], args[0]);
+        } else if (args[3] == 8 && args[4] == 8) {
+            /* load bits 8..15 */
+            tcg_out_modrm(s, OPC_MOVB_EvGv, args[2], args[0] + 4);
+        } else if (args[3] == 0 && args[4] == 16) {
+            /* load bits 0..15 */
+            tcg_out_modrm(s, OPC_MOVL_EvGv | P_DATA16, args[2], args[0]);
+        } else {
+            tcg_abort();
+        }
+        break;
+
     default:
         tcg_abort();
     }

@@ -1802,6 +1822,8 @@ static const TCGTargetOpDef x86_op_defs[] = {
 
     { INDEX_op_setcond_i32, { "q", "r", "ri" } },
 
+    { INDEX_op_deposit_i32, { "Q", "0", "Q" } },
+
 #if TCG_TARGET_REG_BITS == 32
     { INDEX_op_mulu2_i32, { "a", "d", "a", "r" } },
     { INDEX_op_add2_i32, { "r", "r", "0", "1", "ri", "ri" } },

@@ -1853,6 +1875,8 @@ static const TCGTargetOpDef x86_op_defs[] = {
     { INDEX_op_ext8u_i64, { "r", "r" } },
     { INDEX_op_ext16u_i64, { "r", "r" } },
     { INDEX_op_ext32u_i64, { "r", "r" } },
+
+    { INDEX_op_deposit_i64, { "Q", "0", "Q" } },
 #endif
 
 #if TCG_TARGET_REG_BITS == 64

tcg/i386/tcg-target.h +7 -2

index 5088e47..b9c9d4e 100644
@@ -90,7 +90,7 @@ enum {
 #define TCG_TARGET_HAS_eqv_i32          0
 #define TCG_TARGET_HAS_nand_i32         0
 #define TCG_TARGET_HAS_nor_i32          0
-#define TCG_TARGET_HAS_deposit_i32      0
+#define TCG_TARGET_HAS_deposit_i32      1
 
 #if TCG_TARGET_REG_BITS == 64
 #define TCG_TARGET_HAS_div2_i64         1

@@ -111,9 +111,14 @@ enum {
 #define TCG_TARGET_HAS_eqv_i64          0
 #define TCG_TARGET_HAS_nand_i64         0
 #define TCG_TARGET_HAS_nor_i64          0
-#define TCG_TARGET_HAS_deposit_i64      0
+#define TCG_TARGET_HAS_deposit_i64      1
 #endif
 
+#define TCG_TARGET_deposit_i32_valid(ofs, len) \
+    (((ofs) == 0 && (len) == 8) || ((ofs) == 8 && (len) == 8) || \
+     ((ofs) == 0 && (len) == 16))
+#define TCG_TARGET_deposit_i64_valid    TCG_TARGET_deposit_i32_valid
+
 #define TCG_TARGET_HAS_GUEST_BASE
 
 /* Note: must be synced with dyngen-exec.h */

tcg/tcg-op.h +2 -2

index 404b637..fea5983 100644
@@ -2045,7 +2045,7 @@ static inline void tcg_gen_deposit_i32(TCGv_i32 ret, TCGv_i32 arg1,
 				       TCGv_i32 arg2, unsigned int ofs,
 				       unsigned int len)
 {
-    if (TCG_TARGET_HAS_deposit_i32) {
+    if (TCG_TARGET_HAS_deposit_i32 && TCG_TARGET_deposit_i32_valid(ofs, len)) {
         tcg_gen_op5ii_i32(INDEX_op_deposit_i32, ret, arg1, arg2, ofs, len);
     } else {
         uint32_t mask = (1u << len) - 1;

@@ -2064,7 +2064,7 @@ static inline void tcg_gen_deposit_i64(TCGv_i64 ret, TCGv_i64 arg1,
 				       TCGv_i64 arg2, unsigned int ofs,
 				       unsigned int len)
 {
-    if (TCG_TARGET_HAS_deposit_i64) {
+    if (TCG_TARGET_HAS_deposit_i64 && TCG_TARGET_deposit_i64_valid(ofs, len)) {
         tcg_gen_op5ii_i64(INDEX_op_deposit_i64, ret, arg1, arg2, ofs, len);
     } else {
         uint64_t mask = (1ull << len) - 1;

tcg/tcg.h +7 -0

index dc5e9c9..c7197f5 100644
@@ -71,6 +71,13 @@ typedef uint64_t TCGRegSet;
 #define TCG_TARGET_HAS_deposit_i64      0
 #endif
 
+#ifndef TCG_TARGET_deposit_i32_valid
+#define TCG_TARGET_deposit_i32_valid(ofs, len) 1
+#endif
+#ifndef TCG_TARGET_deposit_i64_valid
+#define TCG_TARGET_deposit_i64_valid(ofs, len) 1
+#endif
+
 /* Only one of DIV or DIV2 should be defined.  */
 #if defined(TCG_TARGET_HAS_div_i32)
 #define TCG_TARGET_HAS_div2_i32         0

Signature · 1 line

1.7.3.4

Richard Henderson

Reviewed-by on v4 1/1

2011-09-29 19:50 UTC #12 lore raw

Re: [Qemu-devel] [PATCH v4] tcg-i386: Introduce limited deposit support

Quoted 21 lines · Jan Kiszka

x86 cannot provide an optimized generic deposit implementation. But at least for a few special cases, namely for writing bits 0..7, 8..15, and 0..15, versions using only a single instruction are feasible. Introducing such limited support improves emulating 16-bit x86 code on x86, but also rarer cases where 32-bit or 64-bit code accesses bytes or words.
Signed-off-by Jan Kiszka <[email protected]>
---
Changes in v4:
- provide correct default TCG_TARGET_deposit_i32_valid
Changes in v3:
- provide default TCG_TARGET_deposit_i32_valid - just in case
Changes in v2:
- introduce restricting predicates TCG_TARGET_deposit_i32/64_valid to decide if deposit support can be used
- express register constraints via new 'Q' symbol

Quoted 5 lines

 tcg/i386/tcg-target.c |   24 ++++++++++++++++++++++++
 tcg/i386/tcg-target.h |    9 +++++++--
 tcg/tcg-op.h          |    4 ++--
 tcg/tcg.h             |    7 +++++++
 4 files changed, 40 insertions(+), 4 deletions(-)

Reviewed-by Richard Henderson <[email protected]>

Blue Swirl

Applied

2011-10-01 12:06 UTC #13 lore raw

Re: [Qemu-devel] [PATCH v4] tcg-i386: Introduce limited deposit support

In reply to Jan Kiszka

Thanks, applied.

Quoted 171 lines · Jan Kiszka

Quoted 10 lines · Richard Henderson

tcg/tcg.h typedef uint64_t TCGRegSet; · line 79
Quoted 6 lines · Jan Kiszka
tcg/tcg.h +6 -0
+#ifndef TCG_TARGET_deposit_i32_valid
+#define TCG_TARGET_deposit_i32_valid(ofs, len) 0
+#endif
+#ifndef TCG_TARGET_deposit_i64_valid
+#define TCG_TARGET_deposit_i64_valid(ofs, len) 0
+#endif
Err, no. The default is true. The targets that currently implement deposit at present can handle arbitrary inputs.

Grr, of course.

---

Signed-off-by Jan Kiszka <[email protected]>

---

Changes in v4:
- provide correct default TCG_TARGET_deposit_i32_valid

Changes in v3:
- provide default TCG_TARGET_deposit_i32_valid - just in case

Changes in v2:
- introduce restricting predicates TCG_TARGET_deposit_i32/64_valid to decide if deposit support can be used
- express register constraints via new 'Q' symbol

tcg/i386/tcg-target.c | 24 ++++++++++++++++++++++++ tcg/i386/tcg-target.h | 9 +++++++-- tcg/tcg-op.h | 4 ++--
tcg/tcg.h | 7 +++++++

 4 files changed, 40 insertions(+), 4 deletions(-)

tcg/i386/tcg-target.c +4 -0

index 281f87d..3069e53 100644
@@ -168,6 +168,10 @@ static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
             tcg_regset_set32(ct->u.regs, 0, 0xf);
         }
         break;
+    case 'Q':
+        ct->ct |= TCG_CT_REG;
+        tcg_regset_set32(ct->u.regs, 0, 0xf);
+        break;

case 'r':
ct->ct |= TCG_CT_REG;
if (TCG_TARGET_REG_BITS == 64) {

tcg/i386/tcg-target.c +16 -0

@@ -1747,6 +1751,22 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc,
         break;
  #endif
 
+    OP_32_64(deposit):
+        if (args[3] == 0 && args[4] == 8) {
+            /* load bits 0..7 */
+            tcg_out_modrm(s, OPC_MOVB_EvGv | P_REXB_R | P_REXB_RM,
+                          args[2], args[0]);
+        } else if (args[3] == 8 && args[4] == 8) {
+            /* load bits 8..15 */
+            tcg_out_modrm(s, OPC_MOVB_EvGv, args[2], args[0] + 4);
+        } else if (args[3] == 0 && args[4] == 16) {
+            /* load bits 0..15 */
+            tcg_out_modrm(s, OPC_MOVL_EvGv | P_DATA16, args[2], args[0]);
+        } else {
+            tcg_abort();
+        }
+        break;
+

default:
tcg_abort();
}

tcg/i386/tcg-target.c +4 -0

@@ -1802,6 +1822,8 @@ static const TCGTargetOpDef x86_op_defs[] = {
 
     { INDEX_op_setcond_i32, { "q", "r", "ri" } },
 
+    { INDEX_op_deposit_i32, { "Q", "0", "Q" } },
+
  #if TCG_TARGET_REG_BITS == 32
     { INDEX_op_mulu2_i32, { "a", "d", "a", "r" } },
     { INDEX_op_add2_i32, { "r", "r", "0", "1", "ri", "ri" } },

@@ -1853,6 +1875,8 @@ static const TCGTargetOpDef x86_op_defs[] = {
     { INDEX_op_ext8u_i64, { "r", "r" } },
     { INDEX_op_ext16u_i64, { "r", "r" } },
     { INDEX_op_ext32u_i64, { "r", "r" } },
+
+    { INDEX_op_deposit_i64, { "Q", "0", "Q" } },
  #endif
 
  #if TCG_TARGET_REG_BITS == 64

tcg/i386/tcg-target.h +7 -2

index 5088e47..b9c9d4e 100644
@@ -90,7 +90,7 @@ enum {
  #define TCG_TARGET_HAS_eqv_i32          0
  #define TCG_TARGET_HAS_nand_i32         0
  #define TCG_TARGET_HAS_nor_i32          0
-#define TCG_TARGET_HAS_deposit_i32      0
+#define TCG_TARGET_HAS_deposit_i32      1
 
  #if TCG_TARGET_REG_BITS == 64
  #define TCG_TARGET_HAS_div2_i64         1

@@ -111,9 +111,14 @@ enum {
  #define TCG_TARGET_HAS_eqv_i64          0
  #define TCG_TARGET_HAS_nand_i64         0
  #define TCG_TARGET_HAS_nor_i64          0
-#define TCG_TARGET_HAS_deposit_i64      0
+#define TCG_TARGET_HAS_deposit_i64      1
  #endif
 
+#define TCG_TARGET_deposit_i32_valid(ofs, len) \
+    (((ofs) == 0 && (len) == 8) || ((ofs) == 8 && (len) == 8) || \
+     ((ofs) == 0 && (len) == 16))
+#define TCG_TARGET_deposit_i64_valid    TCG_TARGET_deposit_i32_valid
+
  #define TCG_TARGET_HAS_GUEST_BASE
 
  /* Note: must be synced with dyngen-exec.h */

tcg/tcg-op.h +2 -2

index 404b637..fea5983 100644
@@ -2045,7 +2045,7 @@ static inline void tcg_gen_deposit_i32(TCGv_i32 ret, TCGv_i32 arg1,
                                       TCGv_i32 arg2, unsigned int ofs,
                                       unsigned int len)
  {
-    if (TCG_TARGET_HAS_deposit_i32) {
+    if (TCG_TARGET_HAS_deposit_i32 && TCG_TARGET_deposit_i32_valid(ofs, len)) {
         tcg_gen_op5ii_i32(INDEX_op_deposit_i32, ret, arg1, arg2, ofs, len);
     } else {
         uint32_t mask = (1u << len) - 1;

@@ -2064,7 +2064,7 @@ static inline void tcg_gen_deposit_i64(TCGv_i64 ret, TCGv_i64 arg1,
                                       TCGv_i64 arg2, unsigned int ofs,
                                       unsigned int len)
  {
-    if (TCG_TARGET_HAS_deposit_i64) {
+    if (TCG_TARGET_HAS_deposit_i64 && TCG_TARGET_deposit_i64_valid(ofs, len)) {
         tcg_gen_op5ii_i64(INDEX_op_deposit_i64, ret, arg1, arg2, ofs, len);
     } else {
         uint64_t mask = (1ull << len) - 1;

tcg/tcg.h +7 -1

index dc5e9c9..c7197f5 100644
@@ -71,6 +71,13 @@ typedef uint64_t TCGRegSet;
  #define TCG_TARGET_HAS_deposit_i64      0
  #endif
 
+#ifndef TCG_TARGET_deposit_i32_valid
+#define TCG_TARGET_deposit_i32_valid(ofs, len) 1
+#endif
+#ifndef TCG_TARGET_deposit_i64_valid
+#define TCG_TARGET_deposit_i64_valid(ofs, len) 1
+#endif
+
  /* Only one of DIV or DIV2 should be defined.  */
  #if defined(TCG_TARGET_HAS_div_i32)
  #define TCG_TARGET_HAS_div2_i32         0
--

1.7.3.4