* [Qemu-devel] [PATCH v2 0/4] Fix and clean tcg_target_get_call_iarg_regs_count
@ 2012-09-13 17:37 Stefan Weil
2012-09-13 17:37 ` [Qemu-devel] [PATCH 1/4] w64: Fix TCG helper functions with 5 arguments Stefan Weil
` (4 more replies)
0 siblings, 5 replies; 19+ messages in thread
From: Stefan Weil @ 2012-09-13 17:37 UTC (permalink / raw)
To: Blue Swirl
Cc: Peter Maydell, Alexander Graf, qemu-devel, Aurelien Jarno,
Richard Henderson
These patches replace the 4 patches which I sent yesterday.
Thanks to Aurelien, Peter and Richard for their feedback.
Function tcg_target_get_call_iarg_regs_count was wrong for w64 hosts
and can be removed completely (no more implementations for each TCG target):
[PATCH 1/4] w64: Fix TCG helper functions with 5 arguments (unchanged)
[PATCH 2/4] tcg/i386: Add shortcuts for registers used in L (new)
[PATCH 3/4] tcg/i386: Remove unused registers from (unchanged)
[PATCH 4/4] tcg: Remove tcg_target_get_call_iarg_regs_count (modified)
The first patch is also needed for stable-1.2. It was already sent
yesterday as a single patch.
After the new patch 2, x86 32 bit hosts no longer access
tcg_target_get_call_iarg_regs_count.
Patch 4 not only simplifies tcg_target_get_call_iarg_regs_count but
removes it completely.
Regards,
Stefan Weil
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 1/4] w64: Fix TCG helper functions with 5 arguments
2012-09-13 17:37 [Qemu-devel] [PATCH v2 0/4] Fix and clean tcg_target_get_call_iarg_regs_count Stefan Weil
@ 2012-09-13 17:37 ` Stefan Weil
2012-09-13 21:22 ` Aurelien Jarno
2012-09-22 14:55 ` Aurelien Jarno
2012-09-13 17:37 ` [Qemu-devel] [PATCH 2/4] tcg/i386: Add shortcuts for registers used in L constraint Stefan Weil
` (3 subsequent siblings)
4 siblings, 2 replies; 19+ messages in thread
From: Stefan Weil @ 2012-09-13 17:37 UTC (permalink / raw)
To: Blue Swirl
Cc: Peter Maydell, Stefan Weil, Alexander Graf, qemu-devel,
Aurelien Jarno, Richard Henderson
TCG uses 6 registers for function arguments on 64 bit Linux hosts,
but only 4 registers on W64 hosts.
Commit 2999a0b20074a7e4a58f56572bb1436749368f59 increased the number
of arguments for some important helper functions from 4 to 5
which triggered a bug for W64 hosts: QEMU aborts when executing
helper_lcall_real in the guest's BIOS because function
tcg_target_get_call_iarg_regs_count always returned 6.
As W64 has only 4 registers for arguments, the 5th argument must be
passed on the stack using a correct stack offset.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
tcg/i386/tcg-target.c | 2 +-
tcg/i386/tcg-target.h | 4 ++++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index da17bba..43b5572 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -118,7 +118,7 @@ static void patch_reloc(uint8_t *code_ptr, int type,
static inline int tcg_target_get_call_iarg_regs_count(int flags)
{
if (TCG_TARGET_REG_BITS == 64) {
- return 6;
+ return ARRAY_SIZE(tcg_target_call_iarg_regs);
}
return 0;
diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index c3cfe05..87417d0 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -67,7 +67,11 @@ typedef enum {
/* used for function call generation */
#define TCG_REG_CALL_STACK TCG_REG_ESP
#define TCG_TARGET_STACK_ALIGN 16
+#if defined(_WIN64)
+#define TCG_TARGET_CALL_STACK_OFFSET 32
+#else
#define TCG_TARGET_CALL_STACK_OFFSET 0
+#endif
/* optional instructions */
#define TCG_TARGET_HAS_div2_i32 1
--
1.7.10
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 2/4] tcg/i386: Add shortcuts for registers used in L constraint
2012-09-13 17:37 [Qemu-devel] [PATCH v2 0/4] Fix and clean tcg_target_get_call_iarg_regs_count Stefan Weil
2012-09-13 17:37 ` [Qemu-devel] [PATCH 1/4] w64: Fix TCG helper functions with 5 arguments Stefan Weil
@ 2012-09-13 17:37 ` Stefan Weil
2012-09-13 21:21 ` Aurelien Jarno
2012-09-13 17:37 ` [Qemu-devel] [PATCH 3/4] tcg/i386: Remove unused registers from tcg_target_call_iarg_regs Stefan Weil
` (2 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: Stefan Weil @ 2012-09-13 17:37 UTC (permalink / raw)
To: Blue Swirl
Cc: Peter Maydell, Stefan Weil, Alexander Graf, qemu-devel,
Aurelien Jarno, Richard Henderson
While 64 bit hosts use the first three registers which are also used
as function input parameters, 32 bit hosts use TCG_REG_EAX and
TCG_REG_EDX which are not used in parameter passing.
After defining new register macros for the registers used in L
constraint, the patch replaces most occurrences of
tcg_target_call_iarg_regs[0], tcg_target_call_iarg_regs[1] and
tcg_target_call_iarg_regs[2] by those new macros.
tcg_target_call_iarg_regs remains unchanged when it is used for input
arguments (only with 64 bit hosts) before tcg_out_calli.
A comment related to those registers was fixed, too.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
tcg/i386/tcg-target.c | 84 +++++++++++++++++++++++--------------------------
1 file changed, 40 insertions(+), 44 deletions(-)
diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index 43b5572..ef63967 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -88,6 +88,16 @@ static const int tcg_target_call_oarg_regs[] = {
#endif
};
+/* Registers used with L constraint. */
+#if TCG_TARGET_REG_BITS == 64
+# define TCG_REG_L0 tcg_target_call_iarg_regs[0]
+# define TCG_REG_L1 tcg_target_call_iarg_regs[1]
+# define TCG_REG_L2 tcg_target_call_iarg_regs[2]
+#else
+# define TCG_REG_L0 TCG_REG_EAX
+# define TCG_REG_L1 TCG_REG_EDX
+#endif
+
static uint8_t *tb_ret_addr;
static void patch_reloc(uint8_t *code_ptr, int type,
@@ -181,15 +191,15 @@ static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
ct->ct |= TCG_CT_REG;
if (TCG_TARGET_REG_BITS == 64) {
tcg_regset_set32(ct->u.regs, 0, 0xffff);
- tcg_regset_reset_reg(ct->u.regs, tcg_target_call_iarg_regs[0]);
- tcg_regset_reset_reg(ct->u.regs, tcg_target_call_iarg_regs[1]);
+ tcg_regset_reset_reg(ct->u.regs, TCG_REG_L0);
+ tcg_regset_reset_reg(ct->u.regs, TCG_REG_L1);
#ifdef CONFIG_TCG_PASS_AREG0
- tcg_regset_reset_reg(ct->u.regs, tcg_target_call_iarg_regs[2]);
+ tcg_regset_reset_reg(ct->u.regs, TCG_REG_L2);
#endif
} else {
tcg_regset_set32(ct->u.regs, 0, 0xff);
- tcg_regset_reset_reg(ct->u.regs, TCG_REG_EAX);
- tcg_regset_reset_reg(ct->u.regs, TCG_REG_EDX);
+ tcg_regset_reset_reg(ct->u.regs, TCG_REG_L0);
+ tcg_regset_reset_reg(ct->u.regs, TCG_REG_L1);
}
break;
@@ -1031,8 +1041,8 @@ static inline void tcg_out_tlb_load(TCGContext *s, int addrlo_idx,
uint8_t **label_ptr, int which)
{
const int addrlo = args[addrlo_idx];
- const int r0 = tcg_target_call_iarg_regs[0];
- const int r1 = tcg_target_call_iarg_regs[1];
+ const int r0 = TCG_REG_L0;
+ const int r1 = TCG_REG_L1;
TCGType type = TCG_TYPE_I32;
int rexw = 0;
@@ -1194,8 +1204,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args,
label_ptr, offsetof(CPUTLBEntry, addr_read));
/* TLB Hit. */
- tcg_out_qemu_ld_direct(s, data_reg, data_reg2,
- tcg_target_call_iarg_regs[0], 0, opc);
+ tcg_out_qemu_ld_direct(s, data_reg, data_reg2, TCG_REG_L0, 0, opc);
/* jmp label2 */
tcg_out8(s, OPC_JMP_short);
@@ -1231,14 +1240,10 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args,
mem_index);
#ifdef CONFIG_TCG_PASS_AREG0
/* XXX/FIXME: suboptimal */
- tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[3],
- tcg_target_call_iarg_regs[2]);
- tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2],
- tcg_target_call_iarg_regs[1]);
- tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[1],
- tcg_target_call_iarg_regs[0]);
- tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[0],
- TCG_AREG0);
+ tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[3], TCG_REG_L2);
+ tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2], TCG_REG_L1);
+ tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[1], TCG_REG_L0);
+ tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[0], TCG_AREG0);
#endif
#endif
@@ -1305,11 +1310,9 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args,
use the ADDR32 prefix. For now, do nothing. */
if (offset != GUEST_BASE) {
- tcg_out_movi(s, TCG_TYPE_I64,
- tcg_target_call_iarg_regs[0], GUEST_BASE);
- tgen_arithr(s, ARITH_ADD + P_REXW,
- tcg_target_call_iarg_regs[0], base);
- base = tcg_target_call_iarg_regs[0];
+ tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L0, GUEST_BASE);
+ tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L0, base);
+ base = TCG_REG_L0;
offset = 0;
}
}
@@ -1330,8 +1333,8 @@ static void tcg_out_qemu_st_direct(TCGContext *s, int datalo, int datahi,
/* ??? Ideally we wouldn't need a scratch register. For user-only,
we could perform the bswap twice to restore the original value
instead of moving to the scratch. But as it is, the L constraint
- means that the second argument reg is definitely free here. */
- int scratch = tcg_target_call_iarg_regs[1];
+ means that TCG_REG_L1 is definitely free here. */
+ const int scratch = TCG_REG_L1;
switch (sizeop) {
case 0:
@@ -1404,8 +1407,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args,
label_ptr, offsetof(CPUTLBEntry, addr_write));
/* TLB Hit. */
- tcg_out_qemu_st_direct(s, data_reg, data_reg2,
- tcg_target_call_iarg_regs[0], 0, opc);
+ tcg_out_qemu_st_direct(s, data_reg, data_reg2, TCG_REG_L0, 0, opc);
/* jmp label2 */
tcg_out8(s, OPC_JMP_short);
@@ -1442,19 +1444,15 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args,
#endif
#else
tcg_out_mov(s, (opc == 3 ? TCG_TYPE_I64 : TCG_TYPE_I32),
- tcg_target_call_iarg_regs[1], data_reg);
- tcg_out_movi(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[2], mem_index);
+ TCG_REG_L1, data_reg);
+ tcg_out_movi(s, TCG_TYPE_I32, TCG_REG_L2, mem_index);
stack_adjust = 0;
#ifdef CONFIG_TCG_PASS_AREG0
/* XXX/FIXME: suboptimal */
- tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[3],
- tcg_target_call_iarg_regs[2]);
- tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2],
- tcg_target_call_iarg_regs[1]);
- tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[1],
- tcg_target_call_iarg_regs[0]);
- tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[0],
- TCG_AREG0);
+ tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[3], TCG_REG_L2);
+ tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2], TCG_REG_L1);
+ tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[1], TCG_REG_L0);
+ tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[0], TCG_AREG0);
#endif
#endif
@@ -1482,11 +1480,9 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args,
use the ADDR32 prefix. For now, do nothing. */
if (offset != GUEST_BASE) {
- tcg_out_movi(s, TCG_TYPE_I64,
- tcg_target_call_iarg_regs[0], GUEST_BASE);
- tgen_arithr(s, ARITH_ADD + P_REXW,
- tcg_target_call_iarg_regs[0], base);
- base = tcg_target_call_iarg_regs[0];
+ tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L0, GUEST_BASE);
+ tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L0, base);
+ base = TCG_REG_L0;
offset = 0;
}
}
@@ -2038,15 +2034,15 @@ static void tcg_target_qemu_prologue(TCGContext *s)
#if TCG_TARGET_REG_BITS == 32
tcg_out_ld(s, TCG_TYPE_PTR, TCG_AREG0, TCG_REG_ESP,
(ARRAY_SIZE(tcg_target_callee_save_regs) + 1) * 4);
- tcg_out_ld(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[1], TCG_REG_ESP,
+ tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_L1, TCG_REG_ESP,
(ARRAY_SIZE(tcg_target_callee_save_regs) + 2) * 4);
#else
- tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, tcg_target_call_iarg_regs[0]);
+ tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, TCG_REG_L0);
#endif
tcg_out_addi(s, TCG_REG_ESP, -stack_addend);
/* jmp *tb. */
- tcg_out_modrm(s, OPC_GRP5, EXT5_JMPN_Ev, tcg_target_call_iarg_regs[1]);
+ tcg_out_modrm(s, OPC_GRP5, EXT5_JMPN_Ev, TCG_REG_L1);
/* TB epilogue */
tb_ret_addr = s->code_ptr;
--
1.7.10
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 3/4] tcg/i386: Remove unused registers from tcg_target_call_iarg_regs
2012-09-13 17:37 [Qemu-devel] [PATCH v2 0/4] Fix and clean tcg_target_get_call_iarg_regs_count Stefan Weil
2012-09-13 17:37 ` [Qemu-devel] [PATCH 1/4] w64: Fix TCG helper functions with 5 arguments Stefan Weil
2012-09-13 17:37 ` [Qemu-devel] [PATCH 2/4] tcg/i386: Add shortcuts for registers used in L constraint Stefan Weil
@ 2012-09-13 17:37 ` Stefan Weil
2012-09-13 21:22 ` Aurelien Jarno
2012-09-22 14:55 ` Aurelien Jarno
2012-09-13 17:37 ` [Qemu-devel] [PATCH 4/4] tcg: Remove tcg_target_get_call_iarg_regs_count Stefan Weil
2012-09-13 17:52 ` [Qemu-devel] [PATCH v2 0/4] Fix and clean tcg_target_get_call_iarg_regs_count Richard Henderson
4 siblings, 2 replies; 19+ messages in thread
From: Stefan Weil @ 2012-09-13 17:37 UTC (permalink / raw)
To: Blue Swirl
Cc: Peter Maydell, Stefan Weil, Alexander Graf, qemu-devel,
Aurelien Jarno, Richard Henderson
32 bit x86 hosts don't need registers for helper function arguments
because they use the default stack based calling convention.
Removing the registers allows simpler code for function
tcg_target_get_call_iarg_regs_count.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
tcg/i386/tcg-target.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index ef63967..375d87d 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -75,9 +75,7 @@ static const int tcg_target_call_iarg_regs[] = {
TCG_REG_R8,
TCG_REG_R9,
#else
- TCG_REG_EAX,
- TCG_REG_EDX,
- TCG_REG_ECX
+ /* 32 bit mode uses stack based calling convention (GCC default). */
#endif
};
@@ -127,11 +125,7 @@ static void patch_reloc(uint8_t *code_ptr, int type,
/* maximum number of register used for input function arguments */
static inline int tcg_target_get_call_iarg_regs_count(int flags)
{
- if (TCG_TARGET_REG_BITS == 64) {
- return ARRAY_SIZE(tcg_target_call_iarg_regs);
- }
-
- return 0;
+ return ARRAY_SIZE(tcg_target_call_iarg_regs);
}
/* parse target specific constraints */
--
1.7.10
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH 4/4] tcg: Remove tcg_target_get_call_iarg_regs_count
2012-09-13 17:37 [Qemu-devel] [PATCH v2 0/4] Fix and clean tcg_target_get_call_iarg_regs_count Stefan Weil
` (2 preceding siblings ...)
2012-09-13 17:37 ` [Qemu-devel] [PATCH 3/4] tcg/i386: Remove unused registers from tcg_target_call_iarg_regs Stefan Weil
@ 2012-09-13 17:37 ` Stefan Weil
2012-09-13 21:22 ` Aurelien Jarno
2012-09-22 14:55 ` Aurelien Jarno
2012-09-13 17:52 ` [Qemu-devel] [PATCH v2 0/4] Fix and clean tcg_target_get_call_iarg_regs_count Richard Henderson
4 siblings, 2 replies; 19+ messages in thread
From: Stefan Weil @ 2012-09-13 17:37 UTC (permalink / raw)
To: Blue Swirl
Cc: Peter Maydell, Stefan Weil, Alexander Graf, qemu-devel,
Aurelien Jarno, Richard Henderson
The TCG targets no longer need individual implementations.
Since commit 6a18ae2d2947532d5c26439548afa0481c4529f9,
'flags' is no longer used in tcg_target_get_call_iarg_regs_count.
The remaining tcg_target_get_call_iarg_regs_count is trivial and only
called once. Therefore the patch eliminates it completely.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
tcg/arm/tcg-target.c | 6 ------
tcg/hppa/tcg-target.c | 6 ------
tcg/i386/tcg-target.c | 6 ------
tcg/ia64/tcg-target.c | 6 ------
tcg/mips/tcg-target.c | 6 ------
tcg/ppc/tcg-target.c | 6 ------
tcg/ppc64/tcg-target.c | 6 ------
tcg/s390/tcg-target.c | 5 -----
tcg/sparc/tcg-target.c | 6 ------
tcg/tcg.c | 3 +--
tcg/tci/tcg-target.c | 6 ------
11 files changed, 1 insertion(+), 61 deletions(-)
diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
index cf0ca3d..38e2057 100644
--- a/tcg/arm/tcg-target.c
+++ b/tcg/arm/tcg-target.c
@@ -145,12 +145,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
}
}
-/* maximum number of register used for input function arguments */
-static inline int tcg_target_get_call_iarg_regs_count(int flags)
-{
- return 4;
-}
-
/* parse target specific constraints */
static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
{
diff --git a/tcg/hppa/tcg-target.c b/tcg/hppa/tcg-target.c
index 4f17928..985476a 100644
--- a/tcg/hppa/tcg-target.c
+++ b/tcg/hppa/tcg-target.c
@@ -175,12 +175,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
*insn_ptr = insn;
}
-/* maximum number of register used for input function arguments */
-static inline int tcg_target_get_call_iarg_regs_count(int flags)
-{
- return 4;
-}
-
/* parse target specific constraints */
static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
{
diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index 375d87d..18c44ff 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -122,12 +122,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
}
}
-/* maximum number of register used for input function arguments */
-static inline int tcg_target_get_call_iarg_regs_count(int flags)
-{
- return ARRAY_SIZE(tcg_target_call_iarg_regs);
-}
-
/* parse target specific constraints */
static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
{
diff --git a/tcg/ia64/tcg-target.c b/tcg/ia64/tcg-target.c
index dc588db..7f31876 100644
--- a/tcg/ia64/tcg-target.c
+++ b/tcg/ia64/tcg-target.c
@@ -176,12 +176,6 @@ static const int tcg_target_call_oarg_regs[] = {
TCG_REG_R8
};
-/* maximum number of register used for input function arguments */
-static inline int tcg_target_get_call_iarg_regs_count(int flags)
-{
- return 8;
-}
-
/*
* opcode formation
*/
diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
index 1006e28..aad590c 100644
--- a/tcg/mips/tcg-target.c
+++ b/tcg/mips/tcg-target.c
@@ -185,12 +185,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
}
}
-/* maximum number of register used for input function arguments */
-static inline int tcg_target_get_call_iarg_regs_count(int flags)
-{
- return 4;
-}
-
/* parse target specific constraints */
static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
{
diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
index 0cff181..b889e34 100644
--- a/tcg/ppc/tcg-target.c
+++ b/tcg/ppc/tcg-target.c
@@ -221,12 +221,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
}
}
-/* maximum number of register used for input function arguments */
-static int tcg_target_get_call_iarg_regs_count(int flags)
-{
- return ARRAY_SIZE (tcg_target_call_iarg_regs);
-}
-
/* parse target specific constraints */
static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
{
diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
index 27a0ae8..08c5c0f 100644
--- a/tcg/ppc64/tcg-target.c
+++ b/tcg/ppc64/tcg-target.c
@@ -208,12 +208,6 @@ static void patch_reloc (uint8_t *code_ptr, int type,
}
}
-/* maximum number of register used for input function arguments */
-static int tcg_target_get_call_iarg_regs_count (int flags)
-{
- return ARRAY_SIZE (tcg_target_call_iarg_regs);
-}
-
/* parse target specific constraints */
static int target_parse_constraint (TCGArgConstraint *ct, const char **pct_str)
{
diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.c
index 99b5339..287f010 100644
--- a/tcg/s390/tcg-target.c
+++ b/tcg/s390/tcg-target.c
@@ -376,11 +376,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
}
}
-static int tcg_target_get_call_iarg_regs_count(int flags)
-{
- return sizeof(tcg_target_call_iarg_regs) / sizeof(int);
-}
-
/* parse target specific constraints */
static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
{
diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
index 247a278..3b6c108 100644
--- a/tcg/sparc/tcg-target.c
+++ b/tcg/sparc/tcg-target.c
@@ -137,12 +137,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
}
}
-/* maximum number of register used for input function arguments */
-static inline int tcg_target_get_call_iarg_regs_count(int flags)
-{
- return 6;
-}
-
/* parse target specific constraints */
static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
{
diff --git a/tcg/tcg.c b/tcg/tcg.c
index a4e7f42..988f25c 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -89,7 +89,6 @@ static void tcg_out_st(TCGContext *s, TCGType type, TCGReg arg, TCGReg arg1,
tcg_target_long arg2);
static int tcg_target_const_match(tcg_target_long val,
const TCGArgConstraint *arg_ct);
-static int tcg_target_get_call_iarg_regs_count(int flags);
TCGOpDef tcg_op_defs[] = {
#define DEF(s, oargs, iargs, cargs, flags) { #s, oargs, iargs, cargs, iargs + oargs + cargs, flags },
@@ -1866,7 +1865,7 @@ static int tcg_reg_alloc_call(TCGContext *s, const TCGOpDef *def,
flags = args[nb_oargs + nb_iargs];
- nb_regs = tcg_target_get_call_iarg_regs_count(flags);
+ nb_regs = ARRAY_SIZE(tcg_target_call_iarg_regs);
if (nb_regs > nb_params)
nb_regs = nb_params;
diff --git a/tcg/tci/tcg-target.c b/tcg/tci/tcg-target.c
index 7124b15..7d43132 100644
--- a/tcg/tci/tcg-target.c
+++ b/tcg/tci/tcg-target.c
@@ -891,12 +891,6 @@ static int tcg_target_const_match(tcg_target_long val,
return arg_ct->ct & TCG_CT_CONST;
}
-/* Maximum number of register used for input function arguments. */
-static int tcg_target_get_call_iarg_regs_count(int flags)
-{
- return ARRAY_SIZE(tcg_target_call_iarg_regs);
-}
-
static void tcg_target_init(TCGContext *s)
{
#if defined(CONFIG_DEBUG_TCG_INTERPRETER)
--
1.7.10
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/4] Fix and clean tcg_target_get_call_iarg_regs_count
2012-09-13 17:37 [Qemu-devel] [PATCH v2 0/4] Fix and clean tcg_target_get_call_iarg_regs_count Stefan Weil
` (3 preceding siblings ...)
2012-09-13 17:37 ` [Qemu-devel] [PATCH 4/4] tcg: Remove tcg_target_get_call_iarg_regs_count Stefan Weil
@ 2012-09-13 17:52 ` Richard Henderson
4 siblings, 0 replies; 19+ messages in thread
From: Richard Henderson @ 2012-09-13 17:52 UTC (permalink / raw)
To: Stefan Weil
Cc: Peter Maydell, qemu-devel, Alexander Graf, Blue Swirl,
Aurelien Jarno
On 09/13/2012 10:37 AM, Stefan Weil wrote:
> [PATCH 1/4] w64: Fix TCG helper functions with 5 arguments (unchanged)
> [PATCH 2/4] tcg/i386: Add shortcuts for registers used in L (new)
> [PATCH 3/4] tcg/i386: Remove unused registers from (unchanged)
> [PATCH 4/4] tcg: Remove tcg_target_get_call_iarg_regs_count (modified)
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] tcg/i386: Add shortcuts for registers used in L constraint
2012-09-13 17:37 ` [Qemu-devel] [PATCH 2/4] tcg/i386: Add shortcuts for registers used in L constraint Stefan Weil
@ 2012-09-13 21:21 ` Aurelien Jarno
2012-09-13 21:30 ` Richard Henderson
0 siblings, 1 reply; 19+ messages in thread
From: Aurelien Jarno @ 2012-09-13 21:21 UTC (permalink / raw)
To: Stefan Weil
Cc: Peter Maydell, Alexander Graf, qemu-devel, Blue Swirl,
Richard Henderson
On Thu, Sep 13, 2012 at 07:37:44PM +0200, Stefan Weil wrote:
> While 64 bit hosts use the first three registers which are also used
> as function input parameters, 32 bit hosts use TCG_REG_EAX and
> TCG_REG_EDX which are not used in parameter passing.
>
> After defining new register macros for the registers used in L
> constraint, the patch replaces most occurrences of
> tcg_target_call_iarg_regs[0], tcg_target_call_iarg_regs[1] and
> tcg_target_call_iarg_regs[2] by those new macros.
>
> tcg_target_call_iarg_regs remains unchanged when it is used for input
> arguments (only with 64 bit hosts) before tcg_out_calli.
>
> A comment related to those registers was fixed, too.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> tcg/i386/tcg-target.c | 84 +++++++++++++++++++++++--------------------------
> 1 file changed, 40 insertions(+), 44 deletions(-)
>
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index 43b5572..ef63967 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -88,6 +88,16 @@ static const int tcg_target_call_oarg_regs[] = {
> #endif
> };
>
> +/* Registers used with L constraint. */
> +#if TCG_TARGET_REG_BITS == 64
> +# define TCG_REG_L0 tcg_target_call_iarg_regs[0]
> +# define TCG_REG_L1 tcg_target_call_iarg_regs[1]
> +# define TCG_REG_L2 tcg_target_call_iarg_regs[2]
> +#else
> +# define TCG_REG_L0 TCG_REG_EAX
> +# define TCG_REG_L1 TCG_REG_EDX
> +#endif
> +
> static uint8_t *tb_ret_addr;
>
> static void patch_reloc(uint8_t *code_ptr, int type,
> @@ -181,15 +191,15 @@ static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> ct->ct |= TCG_CT_REG;
> if (TCG_TARGET_REG_BITS == 64) {
> tcg_regset_set32(ct->u.regs, 0, 0xffff);
> - tcg_regset_reset_reg(ct->u.regs, tcg_target_call_iarg_regs[0]);
> - tcg_regset_reset_reg(ct->u.regs, tcg_target_call_iarg_regs[1]);
> + tcg_regset_reset_reg(ct->u.regs, TCG_REG_L0);
> + tcg_regset_reset_reg(ct->u.regs, TCG_REG_L1);
> #ifdef CONFIG_TCG_PASS_AREG0
> - tcg_regset_reset_reg(ct->u.regs, tcg_target_call_iarg_regs[2]);
> + tcg_regset_reset_reg(ct->u.regs, TCG_REG_L2);
> #endif
> } else {
> tcg_regset_set32(ct->u.regs, 0, 0xff);
> - tcg_regset_reset_reg(ct->u.regs, TCG_REG_EAX);
> - tcg_regset_reset_reg(ct->u.regs, TCG_REG_EDX);
> + tcg_regset_reset_reg(ct->u.regs, TCG_REG_L0);
> + tcg_regset_reset_reg(ct->u.regs, TCG_REG_L1);
> }
> break;
>
> @@ -1031,8 +1041,8 @@ static inline void tcg_out_tlb_load(TCGContext *s, int addrlo_idx,
> uint8_t **label_ptr, int which)
> {
> const int addrlo = args[addrlo_idx];
> - const int r0 = tcg_target_call_iarg_regs[0];
> - const int r1 = tcg_target_call_iarg_regs[1];
> + const int r0 = TCG_REG_L0;
> + const int r1 = TCG_REG_L1;
> TCGType type = TCG_TYPE_I32;
> int rexw = 0;
>
> @@ -1194,8 +1204,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args,
> label_ptr, offsetof(CPUTLBEntry, addr_read));
>
> /* TLB Hit. */
> - tcg_out_qemu_ld_direct(s, data_reg, data_reg2,
> - tcg_target_call_iarg_regs[0], 0, opc);
> + tcg_out_qemu_ld_direct(s, data_reg, data_reg2, TCG_REG_L0, 0, opc);
>
> /* jmp label2 */
> tcg_out8(s, OPC_JMP_short);
> @@ -1231,14 +1240,10 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args,
> mem_index);
> #ifdef CONFIG_TCG_PASS_AREG0
> /* XXX/FIXME: suboptimal */
> - tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[3],
> - tcg_target_call_iarg_regs[2]);
> - tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2],
> - tcg_target_call_iarg_regs[1]);
> - tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[1],
> - tcg_target_call_iarg_regs[0]);
> - tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[0],
> - TCG_AREG0);
> + tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[3], TCG_REG_L2);
> + tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2], TCG_REG_L1);
> + tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[1], TCG_REG_L0);
> + tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[0], TCG_AREG0);
> #endif
> #endif
>
> @@ -1305,11 +1310,9 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg *args,
> use the ADDR32 prefix. For now, do nothing. */
>
> if (offset != GUEST_BASE) {
> - tcg_out_movi(s, TCG_TYPE_I64,
> - tcg_target_call_iarg_regs[0], GUEST_BASE);
> - tgen_arithr(s, ARITH_ADD + P_REXW,
> - tcg_target_call_iarg_regs[0], base);
> - base = tcg_target_call_iarg_regs[0];
> + tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L0, GUEST_BASE);
> + tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L0, base);
> + base = TCG_REG_L0;
> offset = 0;
> }
> }
> @@ -1330,8 +1333,8 @@ static void tcg_out_qemu_st_direct(TCGContext *s, int datalo, int datahi,
> /* ??? Ideally we wouldn't need a scratch register. For user-only,
> we could perform the bswap twice to restore the original value
> instead of moving to the scratch. But as it is, the L constraint
> - means that the second argument reg is definitely free here. */
> - int scratch = tcg_target_call_iarg_regs[1];
> + means that TCG_REG_L1 is definitely free here. */
> + const int scratch = TCG_REG_L1;
>
> switch (sizeop) {
> case 0:
> @@ -1404,8 +1407,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args,
> label_ptr, offsetof(CPUTLBEntry, addr_write));
>
> /* TLB Hit. */
> - tcg_out_qemu_st_direct(s, data_reg, data_reg2,
> - tcg_target_call_iarg_regs[0], 0, opc);
> + tcg_out_qemu_st_direct(s, data_reg, data_reg2, TCG_REG_L0, 0, opc);
>
> /* jmp label2 */
> tcg_out8(s, OPC_JMP_short);
> @@ -1442,19 +1444,15 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args,
> #endif
> #else
> tcg_out_mov(s, (opc == 3 ? TCG_TYPE_I64 : TCG_TYPE_I32),
> - tcg_target_call_iarg_regs[1], data_reg);
> - tcg_out_movi(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[2], mem_index);
> + TCG_REG_L1, data_reg);
> + tcg_out_movi(s, TCG_TYPE_I32, TCG_REG_L2, mem_index);
> stack_adjust = 0;
> #ifdef CONFIG_TCG_PASS_AREG0
> /* XXX/FIXME: suboptimal */
> - tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[3],
> - tcg_target_call_iarg_regs[2]);
> - tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2],
> - tcg_target_call_iarg_regs[1]);
> - tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[1],
> - tcg_target_call_iarg_regs[0]);
> - tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[0],
> - TCG_AREG0);
> + tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[3], TCG_REG_L2);
> + tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2], TCG_REG_L1);
> + tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[1], TCG_REG_L0);
> + tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[0], TCG_AREG0);
> #endif
> #endif
>
> @@ -1482,11 +1480,9 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args,
> use the ADDR32 prefix. For now, do nothing. */
>
> if (offset != GUEST_BASE) {
> - tcg_out_movi(s, TCG_TYPE_I64,
> - tcg_target_call_iarg_regs[0], GUEST_BASE);
> - tgen_arithr(s, ARITH_ADD + P_REXW,
> - tcg_target_call_iarg_regs[0], base);
> - base = tcg_target_call_iarg_regs[0];
> + tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_L0, GUEST_BASE);
> + tgen_arithr(s, ARITH_ADD + P_REXW, TCG_REG_L0, base);
> + base = TCG_REG_L0;
> offset = 0;
> }
> }
> @@ -2038,15 +2034,15 @@ static void tcg_target_qemu_prologue(TCGContext *s)
> #if TCG_TARGET_REG_BITS == 32
> tcg_out_ld(s, TCG_TYPE_PTR, TCG_AREG0, TCG_REG_ESP,
> (ARRAY_SIZE(tcg_target_callee_save_regs) + 1) * 4);
> - tcg_out_ld(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[1], TCG_REG_ESP,
> + tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_L1, TCG_REG_ESP,
> (ARRAY_SIZE(tcg_target_callee_save_regs) + 2) * 4);
> #else
> - tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, tcg_target_call_iarg_regs[0]);
> + tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, TCG_REG_L0);
> #endif
> tcg_out_addi(s, TCG_REG_ESP, -stack_addend);
>
> /* jmp *tb. */
> - tcg_out_modrm(s, OPC_GRP5, EXT5_JMPN_Ev, tcg_target_call_iarg_regs[1]);
> + tcg_out_modrm(s, OPC_GRP5, EXT5_JMPN_Ev, TCG_REG_L1);
I don't think this is correct here. This has nothing to do with the L
constraint, it's really refers to the first and seconds argument passed
to the prologue.
>
> /* TB epilogue */
> tb_ret_addr = s->code_ptr;
> --
> 1.7.10
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] w64: Fix TCG helper functions with 5 arguments
2012-09-13 17:37 ` [Qemu-devel] [PATCH 1/4] w64: Fix TCG helper functions with 5 arguments Stefan Weil
@ 2012-09-13 21:22 ` Aurelien Jarno
2012-09-22 14:55 ` Aurelien Jarno
1 sibling, 0 replies; 19+ messages in thread
From: Aurelien Jarno @ 2012-09-13 21:22 UTC (permalink / raw)
To: Stefan Weil
Cc: Peter Maydell, Alexander Graf, qemu-devel, Blue Swirl,
Richard Henderson
On Thu, Sep 13, 2012 at 07:37:43PM +0200, Stefan Weil wrote:
> TCG uses 6 registers for function arguments on 64 bit Linux hosts,
> but only 4 registers on W64 hosts.
>
> Commit 2999a0b20074a7e4a58f56572bb1436749368f59 increased the number
> of arguments for some important helper functions from 4 to 5
> which triggered a bug for W64 hosts: QEMU aborts when executing
> helper_lcall_real in the guest's BIOS because function
> tcg_target_get_call_iarg_regs_count always returned 6.
>
> As W64 has only 4 registers for arguments, the 5th argument must be
> passed on the stack using a correct stack offset.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> tcg/i386/tcg-target.c | 2 +-
> tcg/i386/tcg-target.h | 4 ++++
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index da17bba..43b5572 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -118,7 +118,7 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> static inline int tcg_target_get_call_iarg_regs_count(int flags)
> {
> if (TCG_TARGET_REG_BITS == 64) {
> - return 6;
> + return ARRAY_SIZE(tcg_target_call_iarg_regs);
> }
>
> return 0;
> diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
> index c3cfe05..87417d0 100644
> --- a/tcg/i386/tcg-target.h
> +++ b/tcg/i386/tcg-target.h
> @@ -67,7 +67,11 @@ typedef enum {
> /* used for function call generation */
> #define TCG_REG_CALL_STACK TCG_REG_ESP
> #define TCG_TARGET_STACK_ALIGN 16
> +#if defined(_WIN64)
> +#define TCG_TARGET_CALL_STACK_OFFSET 32
> +#else
> #define TCG_TARGET_CALL_STACK_OFFSET 0
> +#endif
>
> /* optional instructions */
> #define TCG_TARGET_HAS_div2_i32 1
> --
> 1.7.10
>
>
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] tcg/i386: Remove unused registers from tcg_target_call_iarg_regs
2012-09-13 17:37 ` [Qemu-devel] [PATCH 3/4] tcg/i386: Remove unused registers from tcg_target_call_iarg_regs Stefan Weil
@ 2012-09-13 21:22 ` Aurelien Jarno
2012-09-22 14:55 ` Aurelien Jarno
1 sibling, 0 replies; 19+ messages in thread
From: Aurelien Jarno @ 2012-09-13 21:22 UTC (permalink / raw)
To: Stefan Weil
Cc: Peter Maydell, Alexander Graf, qemu-devel, Blue Swirl,
Richard Henderson
On Thu, Sep 13, 2012 at 07:37:45PM +0200, Stefan Weil wrote:
> 32 bit x86 hosts don't need registers for helper function arguments
> because they use the default stack based calling convention.
>
> Removing the registers allows simpler code for function
> tcg_target_get_call_iarg_regs_count.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> tcg/i386/tcg-target.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index ef63967..375d87d 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -75,9 +75,7 @@ static const int tcg_target_call_iarg_regs[] = {
> TCG_REG_R8,
> TCG_REG_R9,
> #else
> - TCG_REG_EAX,
> - TCG_REG_EDX,
> - TCG_REG_ECX
> + /* 32 bit mode uses stack based calling convention (GCC default). */
> #endif
> };
>
> @@ -127,11 +125,7 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> /* maximum number of register used for input function arguments */
> static inline int tcg_target_get_call_iarg_regs_count(int flags)
> {
> - if (TCG_TARGET_REG_BITS == 64) {
> - return ARRAY_SIZE(tcg_target_call_iarg_regs);
> - }
> -
> - return 0;
> + return ARRAY_SIZE(tcg_target_call_iarg_regs);
> }
>
> /* parse target specific constraints */
> --
> 1.7.10
>
>
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] tcg: Remove tcg_target_get_call_iarg_regs_count
2012-09-13 17:37 ` [Qemu-devel] [PATCH 4/4] tcg: Remove tcg_target_get_call_iarg_regs_count Stefan Weil
@ 2012-09-13 21:22 ` Aurelien Jarno
2012-09-22 14:55 ` Aurelien Jarno
1 sibling, 0 replies; 19+ messages in thread
From: Aurelien Jarno @ 2012-09-13 21:22 UTC (permalink / raw)
To: Stefan Weil
Cc: Peter Maydell, Alexander Graf, qemu-devel, Blue Swirl,
Richard Henderson
On Thu, Sep 13, 2012 at 07:37:46PM +0200, Stefan Weil wrote:
> The TCG targets no longer need individual implementations.
>
> Since commit 6a18ae2d2947532d5c26439548afa0481c4529f9,
> 'flags' is no longer used in tcg_target_get_call_iarg_regs_count.
>
> The remaining tcg_target_get_call_iarg_regs_count is trivial and only
> called once. Therefore the patch eliminates it completely.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> tcg/arm/tcg-target.c | 6 ------
> tcg/hppa/tcg-target.c | 6 ------
> tcg/i386/tcg-target.c | 6 ------
> tcg/ia64/tcg-target.c | 6 ------
> tcg/mips/tcg-target.c | 6 ------
> tcg/ppc/tcg-target.c | 6 ------
> tcg/ppc64/tcg-target.c | 6 ------
> tcg/s390/tcg-target.c | 5 -----
> tcg/sparc/tcg-target.c | 6 ------
> tcg/tcg.c | 3 +--
> tcg/tci/tcg-target.c | 6 ------
> 11 files changed, 1 insertion(+), 61 deletions(-)
>
> diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
> index cf0ca3d..38e2057 100644
> --- a/tcg/arm/tcg-target.c
> +++ b/tcg/arm/tcg-target.c
> @@ -145,12 +145,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> }
> }
>
> -/* maximum number of register used for input function arguments */
> -static inline int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return 4;
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/hppa/tcg-target.c b/tcg/hppa/tcg-target.c
> index 4f17928..985476a 100644
> --- a/tcg/hppa/tcg-target.c
> +++ b/tcg/hppa/tcg-target.c
> @@ -175,12 +175,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> *insn_ptr = insn;
> }
>
> -/* maximum number of register used for input function arguments */
> -static inline int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return 4;
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index 375d87d..18c44ff 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -122,12 +122,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> }
> }
>
> -/* maximum number of register used for input function arguments */
> -static inline int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return ARRAY_SIZE(tcg_target_call_iarg_regs);
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/ia64/tcg-target.c b/tcg/ia64/tcg-target.c
> index dc588db..7f31876 100644
> --- a/tcg/ia64/tcg-target.c
> +++ b/tcg/ia64/tcg-target.c
> @@ -176,12 +176,6 @@ static const int tcg_target_call_oarg_regs[] = {
> TCG_REG_R8
> };
>
> -/* maximum number of register used for input function arguments */
> -static inline int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return 8;
> -}
> -
> /*
> * opcode formation
> */
> diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
> index 1006e28..aad590c 100644
> --- a/tcg/mips/tcg-target.c
> +++ b/tcg/mips/tcg-target.c
> @@ -185,12 +185,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> }
> }
>
> -/* maximum number of register used for input function arguments */
> -static inline int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return 4;
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
> index 0cff181..b889e34 100644
> --- a/tcg/ppc/tcg-target.c
> +++ b/tcg/ppc/tcg-target.c
> @@ -221,12 +221,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> }
> }
>
> -/* maximum number of register used for input function arguments */
> -static int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return ARRAY_SIZE (tcg_target_call_iarg_regs);
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
> index 27a0ae8..08c5c0f 100644
> --- a/tcg/ppc64/tcg-target.c
> +++ b/tcg/ppc64/tcg-target.c
> @@ -208,12 +208,6 @@ static void patch_reloc (uint8_t *code_ptr, int type,
> }
> }
>
> -/* maximum number of register used for input function arguments */
> -static int tcg_target_get_call_iarg_regs_count (int flags)
> -{
> - return ARRAY_SIZE (tcg_target_call_iarg_regs);
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint (TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.c
> index 99b5339..287f010 100644
> --- a/tcg/s390/tcg-target.c
> +++ b/tcg/s390/tcg-target.c
> @@ -376,11 +376,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> }
> }
>
> -static int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return sizeof(tcg_target_call_iarg_regs) / sizeof(int);
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
> index 247a278..3b6c108 100644
> --- a/tcg/sparc/tcg-target.c
> +++ b/tcg/sparc/tcg-target.c
> @@ -137,12 +137,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> }
> }
>
> -/* maximum number of register used for input function arguments */
> -static inline int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return 6;
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index a4e7f42..988f25c 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -89,7 +89,6 @@ static void tcg_out_st(TCGContext *s, TCGType type, TCGReg arg, TCGReg arg1,
> tcg_target_long arg2);
> static int tcg_target_const_match(tcg_target_long val,
> const TCGArgConstraint *arg_ct);
> -static int tcg_target_get_call_iarg_regs_count(int flags);
>
> TCGOpDef tcg_op_defs[] = {
> #define DEF(s, oargs, iargs, cargs, flags) { #s, oargs, iargs, cargs, iargs + oargs + cargs, flags },
> @@ -1866,7 +1865,7 @@ static int tcg_reg_alloc_call(TCGContext *s, const TCGOpDef *def,
>
> flags = args[nb_oargs + nb_iargs];
>
> - nb_regs = tcg_target_get_call_iarg_regs_count(flags);
> + nb_regs = ARRAY_SIZE(tcg_target_call_iarg_regs);
> if (nb_regs > nb_params)
> nb_regs = nb_params;
>
> diff --git a/tcg/tci/tcg-target.c b/tcg/tci/tcg-target.c
> index 7124b15..7d43132 100644
> --- a/tcg/tci/tcg-target.c
> +++ b/tcg/tci/tcg-target.c
> @@ -891,12 +891,6 @@ static int tcg_target_const_match(tcg_target_long val,
> return arg_ct->ct & TCG_CT_CONST;
> }
>
> -/* Maximum number of register used for input function arguments. */
> -static int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return ARRAY_SIZE(tcg_target_call_iarg_regs);
> -}
> -
> static void tcg_target_init(TCGContext *s)
> {
> #if defined(CONFIG_DEBUG_TCG_INTERPRETER)
> --
> 1.7.10
>
>
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] tcg/i386: Add shortcuts for registers used in L constraint
2012-09-13 21:21 ` Aurelien Jarno
@ 2012-09-13 21:30 ` Richard Henderson
2012-09-13 21:47 ` Aurelien Jarno
0 siblings, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2012-09-13 21:30 UTC (permalink / raw)
To: Aurelien Jarno
Cc: Peter Maydell, Stefan Weil, qemu-devel, Alexander Graf,
Blue Swirl
On 09/13/2012 02:21 PM, Aurelien Jarno wrote:
>> > #if TCG_TARGET_REG_BITS == 32
>> > tcg_out_ld(s, TCG_TYPE_PTR, TCG_AREG0, TCG_REG_ESP,
>> > (ARRAY_SIZE(tcg_target_callee_save_regs) + 1) * 4);
>> > - tcg_out_ld(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[1], TCG_REG_ESP,
>> > + tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_L1, TCG_REG_ESP,
>> > (ARRAY_SIZE(tcg_target_callee_save_regs) + 2) * 4);
>> > #else
>> > - tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, tcg_target_call_iarg_regs[0]);
>> > + tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, TCG_REG_L0);
>> > #endif
>> > tcg_out_addi(s, TCG_REG_ESP, -stack_addend);
>> >
>> > /* jmp *tb. */
>> > - tcg_out_modrm(s, OPC_GRP5, EXT5_JMPN_Ev, tcg_target_call_iarg_regs[1]);
>> > + tcg_out_modrm(s, OPC_GRP5, EXT5_JMPN_Ev, TCG_REG_L1);
> I don't think this is correct here. This has nothing to do with the L
> constraint, it's really refers to the first and seconds argument passed
> to the prologue.
>
In the 32-bit case, our use of TCG_REG_L1 really is just a temporary.
We loaded it from the stack just above there.
For the 64-bit case you're right. But that's exactly how we set up
the Ln macros, so I think that's ok.
r~
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] tcg/i386: Add shortcuts for registers used in L constraint
2012-09-13 21:30 ` Richard Henderson
@ 2012-09-13 21:47 ` Aurelien Jarno
2012-09-13 22:03 ` Peter Maydell
0 siblings, 1 reply; 19+ messages in thread
From: Aurelien Jarno @ 2012-09-13 21:47 UTC (permalink / raw)
To: Richard Henderson
Cc: Blue Swirl, Peter Maydell, Alexander Graf, qemu-devel,
Stefan Weil
On Thu, Sep 13, 2012 at 02:30:36PM -0700, Richard Henderson wrote:
> On 09/13/2012 02:21 PM, Aurelien Jarno wrote:
> >> > #if TCG_TARGET_REG_BITS == 32
> >> > tcg_out_ld(s, TCG_TYPE_PTR, TCG_AREG0, TCG_REG_ESP,
> >> > (ARRAY_SIZE(tcg_target_callee_save_regs) + 1) * 4);
> >> > - tcg_out_ld(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[1], TCG_REG_ESP,
> >> > + tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_L1, TCG_REG_ESP,
> >> > (ARRAY_SIZE(tcg_target_callee_save_regs) + 2) * 4);
> >> > #else
> >> > - tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, tcg_target_call_iarg_regs[0]);
> >> > + tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, TCG_REG_L0);
> >> > #endif
> >> > tcg_out_addi(s, TCG_REG_ESP, -stack_addend);
> >> >
> >> > /* jmp *tb. */
> >> > - tcg_out_modrm(s, OPC_GRP5, EXT5_JMPN_Ev, tcg_target_call_iarg_regs[1]);
> >> > + tcg_out_modrm(s, OPC_GRP5, EXT5_JMPN_Ev, TCG_REG_L1);
> > I don't think this is correct here. This has nothing to do with the L
> > constraint, it's really refers to the first and seconds argument passed
> > to the prologue.
> >
>
> In the 32-bit case, our use of TCG_REG_L1 really is just a temporary.
> We loaded it from the stack just above there.
Yeah, I missed this one. This should probably be replaced directly by
the name of the register.
> For the 64-bit case you're right. But that's exactly how we set up
> the Ln macros, so I think that's ok.
>
Then we should change the name and especially the comment above it. They
are just #define for the first argument registers, and not related to
the 'L' constraint.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] tcg/i386: Add shortcuts for registers used in L constraint
2012-09-13 21:47 ` Aurelien Jarno
@ 2012-09-13 22:03 ` Peter Maydell
2012-09-13 22:20 ` Richard Henderson
0 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2012-09-13 22:03 UTC (permalink / raw)
To: Aurelien Jarno
Cc: Blue Swirl, Stefan Weil, Alexander Graf, qemu-devel,
Richard Henderson
On 13 September 2012 22:47, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Thu, Sep 13, 2012 at 02:30:36PM -0700, Richard Henderson wrote:
>> On 09/13/2012 02:21 PM, Aurelien Jarno wrote:
>> >> > #if TCG_TARGET_REG_BITS == 32
>> >> > tcg_out_ld(s, TCG_TYPE_PTR, TCG_AREG0, TCG_REG_ESP,
>> >> > (ARRAY_SIZE(tcg_target_callee_save_regs) + 1) * 4);
>> >> > - tcg_out_ld(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[1], TCG_REG_ESP,
>> >> > + tcg_out_ld(s, TCG_TYPE_PTR, TCG_REG_L1, TCG_REG_ESP,
>> >> > (ARRAY_SIZE(tcg_target_callee_save_regs) + 2) * 4);
>> >> > #else
>> >> > - tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, tcg_target_call_iarg_regs[0]);
>> >> > + tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, TCG_REG_L0);
>> >> > #endif
>> >> > tcg_out_addi(s, TCG_REG_ESP, -stack_addend);
>> >> >
>> >> > /* jmp *tb. */
>> >> > - tcg_out_modrm(s, OPC_GRP5, EXT5_JMPN_Ev, tcg_target_call_iarg_regs[1]);
>> >> > + tcg_out_modrm(s, OPC_GRP5, EXT5_JMPN_Ev, TCG_REG_L1);
>> > I don't think this is correct here. This has nothing to do with the L
>> > constraint, it's really refers to the first and seconds argument passed
>> > to the prologue.
>> >
>>
>> In the 32-bit case, our use of TCG_REG_L1 really is just a temporary.
>> We loaded it from the stack just above there.
>
> Yeah, I missed this one. This should probably be replaced directly by
> the name of the register.
That would mean you'd have to #ifdef it though, since the register you
want is "random temporary for 32 bits, but 2nd input argument register
for 64 bits". (Or you could just use TCG_REG_L1.)
>> For the 64-bit case you're right. But that's exactly how we set up
>> the Ln macros, so I think that's ok.
>>
>
> Then we should change the name and especially the comment above it. They
> are just #define for the first argument registers, and not related to
> the 'L' constraint.
Hard to come up with a snappy name for "register which is the Nth
input argument if input args are in registers, but an arbitrary
temp reg otherwise, and which is in the forbidden list for the L
constraint"...
The alternative to that would be to pull out the 32 and 64 bit
cases rather than trying to share as much of the generation
code as we do at the moment. That might be clearer to read
but it would be a larger change.
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] tcg/i386: Add shortcuts for registers used in L constraint
2012-09-13 22:03 ` Peter Maydell
@ 2012-09-13 22:20 ` Richard Henderson
2012-09-14 5:18 ` Stefan Weil
0 siblings, 1 reply; 19+ messages in thread
From: Richard Henderson @ 2012-09-13 22:20 UTC (permalink / raw)
To: Peter Maydell
Cc: Blue Swirl, Stefan Weil, qemu-devel, Aurelien Jarno,
Alexander Graf
On 09/13/2012 03:03 PM, Peter Maydell wrote:
> Hard to come up with a snappy name for "register which is the Nth
> input argument if input args are in registers, but an arbitrary
> temp reg otherwise, and which is in the forbidden list for the L
> constraint"...
I'm more than happy to let "register which is ..." be a comment
just before the definitions of TCG_REG_Ln at the top...
r~
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] tcg/i386: Add shortcuts for registers used in L constraint
2012-09-13 22:20 ` Richard Henderson
@ 2012-09-14 5:18 ` Stefan Weil
2012-09-22 14:55 ` Aurelien Jarno
0 siblings, 1 reply; 19+ messages in thread
From: Stefan Weil @ 2012-09-14 5:18 UTC (permalink / raw)
To: Richard Henderson
Cc: Blue Swirl, Peter Maydell, qemu-devel, Aurelien Jarno,
Alexander Graf
Am 14.09.2012 00:20, schrieb Richard Henderson:
> On 09/13/2012 03:03 PM, Peter Maydell wrote:
>> Hard to come up with a snappy name for "register which is the Nth
>> input argument if input args are in registers, but an arbitrary
>> temp reg otherwise, and which is in the forbidden list for the L
>> constraint"...
> I'm more than happy to let "register which is ..." be a comment
> just before the definitions of TCG_REG_Ln at the top...
>
>
>
> r~
>
This looks like a pragmatic solution.
If everybody agrees, I'll send an updated patch.
Or Aurelien or Blue can commit this series and fix the comment
as suggested by Richard. Maybe a comment in theprologue code
would also be good.
Thanks to all reviewers.
Stefan
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 1/4] w64: Fix TCG helper functions with 5 arguments
2012-09-13 17:37 ` [Qemu-devel] [PATCH 1/4] w64: Fix TCG helper functions with 5 arguments Stefan Weil
2012-09-13 21:22 ` Aurelien Jarno
@ 2012-09-22 14:55 ` Aurelien Jarno
1 sibling, 0 replies; 19+ messages in thread
From: Aurelien Jarno @ 2012-09-22 14:55 UTC (permalink / raw)
To: Stefan Weil
Cc: Peter Maydell, Alexander Graf, qemu-devel, Blue Swirl,
Richard Henderson
On Thu, Sep 13, 2012 at 07:37:43PM +0200, Stefan Weil wrote:
> TCG uses 6 registers for function arguments on 64 bit Linux hosts,
> but only 4 registers on W64 hosts.
>
> Commit 2999a0b20074a7e4a58f56572bb1436749368f59 increased the number
> of arguments for some important helper functions from 4 to 5
> which triggered a bug for W64 hosts: QEMU aborts when executing
> helper_lcall_real in the guest's BIOS because function
> tcg_target_get_call_iarg_regs_count always returned 6.
>
> As W64 has only 4 registers for arguments, the 5th argument must be
> passed on the stack using a correct stack offset.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> tcg/i386/tcg-target.c | 2 +-
> tcg/i386/tcg-target.h | 4 ++++
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index da17bba..43b5572 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -118,7 +118,7 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> static inline int tcg_target_get_call_iarg_regs_count(int flags)
> {
> if (TCG_TARGET_REG_BITS == 64) {
> - return 6;
> + return ARRAY_SIZE(tcg_target_call_iarg_regs);
> }
>
> return 0;
> diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
> index c3cfe05..87417d0 100644
> --- a/tcg/i386/tcg-target.h
> +++ b/tcg/i386/tcg-target.h
> @@ -67,7 +67,11 @@ typedef enum {
> /* used for function call generation */
> #define TCG_REG_CALL_STACK TCG_REG_ESP
> #define TCG_TARGET_STACK_ALIGN 16
> +#if defined(_WIN64)
> +#define TCG_TARGET_CALL_STACK_OFFSET 32
> +#else
> #define TCG_TARGET_CALL_STACK_OFFSET 0
> +#endif
>
> /* optional instructions */
> #define TCG_TARGET_HAS_div2_i32 1
> --
> 1.7.10
>
>
Thanks, applied.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] tcg/i386: Remove unused registers from tcg_target_call_iarg_regs
2012-09-13 17:37 ` [Qemu-devel] [PATCH 3/4] tcg/i386: Remove unused registers from tcg_target_call_iarg_regs Stefan Weil
2012-09-13 21:22 ` Aurelien Jarno
@ 2012-09-22 14:55 ` Aurelien Jarno
1 sibling, 0 replies; 19+ messages in thread
From: Aurelien Jarno @ 2012-09-22 14:55 UTC (permalink / raw)
To: Stefan Weil
Cc: Blue Swirl, Peter Maydell, Richard Henderson, Alexander Graf,
qemu-devel
On Thu, Sep 13, 2012 at 07:37:45PM +0200, Stefan Weil wrote:
> 32 bit x86 hosts don't need registers for helper function arguments
> because they use the default stack based calling convention.
>
> Removing the registers allows simpler code for function
> tcg_target_get_call_iarg_regs_count.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> tcg/i386/tcg-target.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index ef63967..375d87d 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -75,9 +75,7 @@ static const int tcg_target_call_iarg_regs[] = {
> TCG_REG_R8,
> TCG_REG_R9,
> #else
> - TCG_REG_EAX,
> - TCG_REG_EDX,
> - TCG_REG_ECX
> + /* 32 bit mode uses stack based calling convention (GCC default). */
> #endif
> };
>
> @@ -127,11 +125,7 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> /* maximum number of register used for input function arguments */
> static inline int tcg_target_get_call_iarg_regs_count(int flags)
> {
> - if (TCG_TARGET_REG_BITS == 64) {
> - return ARRAY_SIZE(tcg_target_call_iarg_regs);
> - }
> -
> - return 0;
> + return ARRAY_SIZE(tcg_target_call_iarg_regs);
> }
>
> /* parse target specific constraints */
> --
> 1.7.10
>
>
Thanks, applied.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] tcg: Remove tcg_target_get_call_iarg_regs_count
2012-09-13 17:37 ` [Qemu-devel] [PATCH 4/4] tcg: Remove tcg_target_get_call_iarg_regs_count Stefan Weil
2012-09-13 21:22 ` Aurelien Jarno
@ 2012-09-22 14:55 ` Aurelien Jarno
1 sibling, 0 replies; 19+ messages in thread
From: Aurelien Jarno @ 2012-09-22 14:55 UTC (permalink / raw)
To: Stefan Weil
Cc: Peter Maydell, Alexander Graf, qemu-devel, Blue Swirl,
Richard Henderson
On Thu, Sep 13, 2012 at 07:37:46PM +0200, Stefan Weil wrote:
> The TCG targets no longer need individual implementations.
>
> Since commit 6a18ae2d2947532d5c26439548afa0481c4529f9,
> 'flags' is no longer used in tcg_target_get_call_iarg_regs_count.
>
> The remaining tcg_target_get_call_iarg_regs_count is trivial and only
> called once. Therefore the patch eliminates it completely.
>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> tcg/arm/tcg-target.c | 6 ------
> tcg/hppa/tcg-target.c | 6 ------
> tcg/i386/tcg-target.c | 6 ------
> tcg/ia64/tcg-target.c | 6 ------
> tcg/mips/tcg-target.c | 6 ------
> tcg/ppc/tcg-target.c | 6 ------
> tcg/ppc64/tcg-target.c | 6 ------
> tcg/s390/tcg-target.c | 5 -----
> tcg/sparc/tcg-target.c | 6 ------
> tcg/tcg.c | 3 +--
> tcg/tci/tcg-target.c | 6 ------
> 11 files changed, 1 insertion(+), 61 deletions(-)
>
> diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
> index cf0ca3d..38e2057 100644
> --- a/tcg/arm/tcg-target.c
> +++ b/tcg/arm/tcg-target.c
> @@ -145,12 +145,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> }
> }
>
> -/* maximum number of register used for input function arguments */
> -static inline int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return 4;
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/hppa/tcg-target.c b/tcg/hppa/tcg-target.c
> index 4f17928..985476a 100644
> --- a/tcg/hppa/tcg-target.c
> +++ b/tcg/hppa/tcg-target.c
> @@ -175,12 +175,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> *insn_ptr = insn;
> }
>
> -/* maximum number of register used for input function arguments */
> -static inline int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return 4;
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index 375d87d..18c44ff 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -122,12 +122,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> }
> }
>
> -/* maximum number of register used for input function arguments */
> -static inline int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return ARRAY_SIZE(tcg_target_call_iarg_regs);
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/ia64/tcg-target.c b/tcg/ia64/tcg-target.c
> index dc588db..7f31876 100644
> --- a/tcg/ia64/tcg-target.c
> +++ b/tcg/ia64/tcg-target.c
> @@ -176,12 +176,6 @@ static const int tcg_target_call_oarg_regs[] = {
> TCG_REG_R8
> };
>
> -/* maximum number of register used for input function arguments */
> -static inline int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return 8;
> -}
> -
> /*
> * opcode formation
> */
> diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
> index 1006e28..aad590c 100644
> --- a/tcg/mips/tcg-target.c
> +++ b/tcg/mips/tcg-target.c
> @@ -185,12 +185,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> }
> }
>
> -/* maximum number of register used for input function arguments */
> -static inline int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return 4;
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
> index 0cff181..b889e34 100644
> --- a/tcg/ppc/tcg-target.c
> +++ b/tcg/ppc/tcg-target.c
> @@ -221,12 +221,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> }
> }
>
> -/* maximum number of register used for input function arguments */
> -static int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return ARRAY_SIZE (tcg_target_call_iarg_regs);
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
> index 27a0ae8..08c5c0f 100644
> --- a/tcg/ppc64/tcg-target.c
> +++ b/tcg/ppc64/tcg-target.c
> @@ -208,12 +208,6 @@ static void patch_reloc (uint8_t *code_ptr, int type,
> }
> }
>
> -/* maximum number of register used for input function arguments */
> -static int tcg_target_get_call_iarg_regs_count (int flags)
> -{
> - return ARRAY_SIZE (tcg_target_call_iarg_regs);
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint (TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.c
> index 99b5339..287f010 100644
> --- a/tcg/s390/tcg-target.c
> +++ b/tcg/s390/tcg-target.c
> @@ -376,11 +376,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> }
> }
>
> -static int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return sizeof(tcg_target_call_iarg_regs) / sizeof(int);
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
> index 247a278..3b6c108 100644
> --- a/tcg/sparc/tcg-target.c
> +++ b/tcg/sparc/tcg-target.c
> @@ -137,12 +137,6 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> }
> }
>
> -/* maximum number of register used for input function arguments */
> -static inline int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return 6;
> -}
> -
> /* parse target specific constraints */
> static int target_parse_constraint(TCGArgConstraint *ct, const char **pct_str)
> {
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index a4e7f42..988f25c 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -89,7 +89,6 @@ static void tcg_out_st(TCGContext *s, TCGType type, TCGReg arg, TCGReg arg1,
> tcg_target_long arg2);
> static int tcg_target_const_match(tcg_target_long val,
> const TCGArgConstraint *arg_ct);
> -static int tcg_target_get_call_iarg_regs_count(int flags);
>
> TCGOpDef tcg_op_defs[] = {
> #define DEF(s, oargs, iargs, cargs, flags) { #s, oargs, iargs, cargs, iargs + oargs + cargs, flags },
> @@ -1866,7 +1865,7 @@ static int tcg_reg_alloc_call(TCGContext *s, const TCGOpDef *def,
>
> flags = args[nb_oargs + nb_iargs];
>
> - nb_regs = tcg_target_get_call_iarg_regs_count(flags);
> + nb_regs = ARRAY_SIZE(tcg_target_call_iarg_regs);
> if (nb_regs > nb_params)
> nb_regs = nb_params;
>
> diff --git a/tcg/tci/tcg-target.c b/tcg/tci/tcg-target.c
> index 7124b15..7d43132 100644
> --- a/tcg/tci/tcg-target.c
> +++ b/tcg/tci/tcg-target.c
> @@ -891,12 +891,6 @@ static int tcg_target_const_match(tcg_target_long val,
> return arg_ct->ct & TCG_CT_CONST;
> }
>
> -/* Maximum number of register used for input function arguments. */
> -static int tcg_target_get_call_iarg_regs_count(int flags)
> -{
> - return ARRAY_SIZE(tcg_target_call_iarg_regs);
> -}
> -
> static void tcg_target_init(TCGContext *s)
> {
> #if defined(CONFIG_DEBUG_TCG_INTERPRETER)
> --
> 1.7.10
>
>
Thanks, applied.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] tcg/i386: Add shortcuts for registers used in L constraint
2012-09-14 5:18 ` Stefan Weil
@ 2012-09-22 14:55 ` Aurelien Jarno
0 siblings, 0 replies; 19+ messages in thread
From: Aurelien Jarno @ 2012-09-22 14:55 UTC (permalink / raw)
To: Stefan Weil
Cc: Blue Swirl, Peter Maydell, Alexander Graf, qemu-devel,
Richard Henderson
On Fri, Sep 14, 2012 at 07:18:17AM +0200, Stefan Weil wrote:
> Am 14.09.2012 00:20, schrieb Richard Henderson:
> >On 09/13/2012 03:03 PM, Peter Maydell wrote:
> >>Hard to come up with a snappy name for "register which is the Nth
> >>input argument if input args are in registers, but an arbitrary
> >>temp reg otherwise, and which is in the forbidden list for the L
> >>constraint"...
> >I'm more than happy to let "register which is ..." be a comment
> >just before the definitions of TCG_REG_Ln at the top...
> >
> >
> >
> >r~
> >
>
> This looks like a pragmatic solution.
>
> If everybody agrees, I'll send an updated patch.
>
> Or Aurelien or Blue can commit this series and fix the comment
> as suggested by Richard. Maybe a comment in theprologue code
> would also be good.
>
> Thanks to all reviewers.
>
Your patch didn't even compiled on i386. I have fixed it. I also changed
the prologue on i386 to not load the TB address through a register,
given i386 has so many addressing mode. This way the issue is solved.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-09-22 14:55 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-13 17:37 [Qemu-devel] [PATCH v2 0/4] Fix and clean tcg_target_get_call_iarg_regs_count Stefan Weil
2012-09-13 17:37 ` [Qemu-devel] [PATCH 1/4] w64: Fix TCG helper functions with 5 arguments Stefan Weil
2012-09-13 21:22 ` Aurelien Jarno
2012-09-22 14:55 ` Aurelien Jarno
2012-09-13 17:37 ` [Qemu-devel] [PATCH 2/4] tcg/i386: Add shortcuts for registers used in L constraint Stefan Weil
2012-09-13 21:21 ` Aurelien Jarno
2012-09-13 21:30 ` Richard Henderson
2012-09-13 21:47 ` Aurelien Jarno
2012-09-13 22:03 ` Peter Maydell
2012-09-13 22:20 ` Richard Henderson
2012-09-14 5:18 ` Stefan Weil
2012-09-22 14:55 ` Aurelien Jarno
2012-09-13 17:37 ` [Qemu-devel] [PATCH 3/4] tcg/i386: Remove unused registers from tcg_target_call_iarg_regs Stefan Weil
2012-09-13 21:22 ` Aurelien Jarno
2012-09-22 14:55 ` Aurelien Jarno
2012-09-13 17:37 ` [Qemu-devel] [PATCH 4/4] tcg: Remove tcg_target_get_call_iarg_regs_count Stefan Weil
2012-09-13 21:22 ` Aurelien Jarno
2012-09-22 14:55 ` Aurelien Jarno
2012-09-13 17:52 ` [Qemu-devel] [PATCH v2 0/4] Fix and clean tcg_target_get_call_iarg_regs_count Richard Henderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).