* [Qemu-devel] [PATCH v2 1/7] exec: Reorganize the GETRA/GETPC macros
2013-08-29 22:05 [Qemu-devel] [PATCH v2 0/7] Further tcg ldst improvements Richard Henderson
@ 2013-08-29 22:05 ` Richard Henderson
2013-08-30 16:55 ` Aurelien Jarno
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 2/7] tcg-i386: Don't perform GETPC adjustment in TCG code Richard Henderson
` (5 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2013-08-29 22:05 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien
Always define GETRA; use __builtin_extract_return_addr, rather than
having a special case for s390. Split GETPC_ADJ out of GETPC; use 2
universally, rather than having a special case for arm.
Rename GETPC_LDST to GETRA_LDST to indicate that it does not
contain the GETPC_ADJ value. Likewise with GETPC_EXT to GETRA_EXT.
Perform the GETPC_ADJ adjustment inside helper_ret_ld/st. This will
allow backends to pass along the "true" return address rather than
the massaged GETPC value. In the meantime, double application of
GETPC_ADJ does not hurt, since the call insn in all ISAs is at least
4 bytes long.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
include/exec/exec-all.h | 84 +++++++++++++++++++----------------------
include/exec/softmmu_template.h | 24 ++++++++----
2 files changed, 56 insertions(+), 52 deletions(-)
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index ffb69a4..6f71a4f 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -295,47 +295,42 @@ static inline void tb_add_jump(TranslationBlock *tb, int n,
}
}
-/* The return address may point to the start of the next instruction.
- Subtracting one gets us the call instruction itself. */
+/* GETRA is the true target of the return instruction that we'll execute,
+ defined here for simplicity of defining the follow-up macros. */
#if defined(CONFIG_TCG_INTERPRETER)
extern uintptr_t tci_tb_ptr;
-# define GETPC() tci_tb_ptr
-#elif defined(__s390__) && !defined(__s390x__)
-# define GETPC() \
- (((uintptr_t)__builtin_return_address(0) & 0x7fffffffUL) - 1)
-#elif defined(__arm__)
-/* Thumb return addresses have the low bit set, so we need to subtract two.
- This is still safe in ARM mode because instructions are 4 bytes. */
-# define GETPC() ((uintptr_t)__builtin_return_address(0) - 2)
+# define GETRA() tci_tb_ptr
+#else
+# define GETRA() \
+ ((uintptr_t)__builtin_extract_return_addr(__builtin_return_address(0)))
+#endif
+
+/* The true return address will often point to a host insn that is part of
+ the next translated guest insn. Adjust the address backward to point to
+ the middle of the call insn. Subtracting one would do the job except for
+ several compressed mode architectures (arm, mips) which set the low bit
+ to indicate the compressed mode; subtracting two works around that. It
+ is also the case that there are no host isas that contain a call insn
+ smaller than 4 bytes, so we don't worry about special-casing this. */
+#if defined(CONFIG_TCG_INTERPRETER)
+# define GETPC_ADJ 0
#else
-# define GETPC() ((uintptr_t)__builtin_return_address(0) - 1)
+# define GETPC_ADJ 2
#endif
+#define GETPC() (GETRA() - GETPC_ADJ)
+
+/* The LDST optimizations splits code generation into fast and slow path.
+ In some implementations, we pass the "logical" return address manually;
+ in others, we must infer the logical return from the true return. */
#if defined(CONFIG_QEMU_LDST_OPTIMIZATION) && defined(CONFIG_SOFTMMU)
-/* qemu_ld/st optimization split code generation to fast and slow path, thus,
- it needs special handling for an MMU helper which is called from the slow
- path, to get the fast path's pc without any additional argument.
- It uses a tricky solution which embeds the fast path pc into the slow path.
-
- Code flow in slow path:
- (1) pre-process
- (2) call MMU helper
- (3) jump to (5)
- (4) fast path information (implementation specific)
- (5) post-process (e.g. stack adjust)
- (6) jump to corresponding code of the next of fast path
- */
-# if defined(__i386__) || defined(__x86_64__)
-# define GETPC_EXT() GETPC()
-# elif defined (_ARCH_PPC) && !defined (_ARCH_PPC64)
-# define GETRA() ((uintptr_t)__builtin_return_address(0))
-# define GETPC_LDST() ((uintptr_t) ((*(int32_t *)(GETRA() - 4)) - 1))
+# if defined (_ARCH_PPC) && !defined (_ARCH_PPC64)
+# define GETRA_LDST(RA) (*(int32_t *)((RA) - 4))
# elif defined(__arm__)
/* We define two insns between the return address and the branch back to
straight-line. Find and decode that branch insn. */
-# define GETRA() ((uintptr_t)__builtin_return_address(0))
-# define GETPC_LDST() tcg_getpc_ldst(GETRA())
-static inline uintptr_t tcg_getpc_ldst(uintptr_t ra)
+# define GETRA_LDST(RA) tcg_getra_ldst(RA)
+static inline uintptr_t tcg_getra_ldst(uintptr_t ra)
{
int32_t b;
ra += 8; /* skip the two insns */
@@ -343,33 +338,32 @@ static inline uintptr_t tcg_getpc_ldst(uintptr_t ra)
b = (b << 8) >> (8 - 2); /* extract the displacement */
ra += 8; /* branches are relative to pc+8 */
ra += b; /* apply the displacement */
- ra -= 4; /* return a pointer into the current opcode,
- not the start of the next opcode */
return ra;
}
# elif defined(__aarch64__)
-# define GETRA() ((uintptr_t)__builtin_return_address(0))
-# define GETPC_LDST() tcg_getpc_ldst(GETRA())
-static inline uintptr_t tcg_getpc_ldst(uintptr_t ra)
+# define GETRA_LDST(RA) tcg_getra_ldst(RA)
+static inline uintptr_t tcg_getra_ldst(uintptr_t ra)
{
int32_t b;
ra += 4; /* skip one instruction */
b = *(int32_t *)ra; /* load the branch insn */
b = (b << 6) >> (6 - 2); /* extract the displacement */
ra += b; /* apply the displacement */
- ra -= 4; /* return a pointer into the current opcode,
- not the start of the next opcode */
return ra;
}
-# else
-# error "CONFIG_QEMU_LDST_OPTIMIZATION needs GETPC_LDST() implementation!"
# endif
+#endif /* CONFIG_QEMU_LDST_OPTIMIZATION */
+
+/* ??? Delete these once they are no longer used. */
bool is_tcg_gen_code(uintptr_t pc_ptr);
-# ifndef GETPC_EXT
-# define GETPC_EXT() (is_tcg_gen_code(GETRA()) ? GETPC_LDST() : GETPC())
-# endif
+#ifdef GETRA_LDST
+# define GETRA_EXT() tcg_getra_ext(GETRA())
+static inline uintptr_t tcg_getra_ext(uintptr_t ra)
+{
+ return is_tcg_gen_code(ra) ? GETRA_LDST(ra) : ra;
+}
#else
-# define GETPC_EXT() GETPC()
+# define GETRA_EXT() GETRA()
#endif
#if !defined(CONFIG_USER_ONLY)
diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
index eaca9e1..2fc6ea3 100644
--- a/include/exec/softmmu_template.h
+++ b/include/exec/softmmu_template.h
@@ -86,6 +86,9 @@ glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env,
target_ulong tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ;
uintptr_t haddr;
+ /* Adjust the given return address. */
+ retaddr -= GETPC_ADJ;
+
/* If the TLB entry is for a different page, reload and try again. */
if ((addr & TARGET_PAGE_MASK)
!= (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) {
@@ -121,10 +124,12 @@ glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env,
#endif
addr1 = addr & ~(DATA_SIZE - 1);
addr2 = addr1 + DATA_SIZE;
- res1 = glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(env, addr1,
- mmu_idx, retaddr);
- res2 = glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(env, addr2,
- mmu_idx, retaddr);
+ /* Note the adjustment at the beginning of the function.
+ Undo that for the recursion. */
+ res1 = glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)
+ (env, addr1, mmu_idx, retaddr + GETPC_ADJ);
+ res2 = glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)
+ (env, addr2, mmu_idx, retaddr + GETPC_ADJ);
shift = (addr & (DATA_SIZE - 1)) * 8;
#ifdef TARGET_WORDS_BIGENDIAN
res = (res1 << shift) | (res2 >> ((DATA_SIZE * 8) - shift));
@@ -150,7 +155,7 @@ glue(glue(helper_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
int mmu_idx)
{
return glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(env, addr, mmu_idx,
- GETPC_EXT());
+ GETRA_EXT());
}
#ifndef SOFTMMU_CODE_ACCESS
@@ -182,6 +187,9 @@ glue(glue(helper_ret_st, SUFFIX), MMUSUFFIX)(CPUArchState *env,
target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write;
uintptr_t haddr;
+ /* Adjust the given return address. */
+ retaddr -= GETPC_ADJ;
+
/* If the TLB entry is for a different page, reload and try again. */
if ((addr & TARGET_PAGE_MASK)
!= (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) {
@@ -223,8 +231,10 @@ glue(glue(helper_ret_st, SUFFIX), MMUSUFFIX)(CPUArchState *env,
#else
uint8_t val8 = val >> (i * 8);
#endif
+ /* Note the adjustment at the beginning of the function.
+ Undo that for the recursion. */
glue(helper_ret_stb, MMUSUFFIX)(env, addr + i, val8,
- mmu_idx, retaddr);
+ mmu_idx, retaddr + GETPC_ADJ);
}
return;
}
@@ -245,7 +255,7 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
DATA_TYPE val, int mmu_idx)
{
glue(glue(helper_ret_st, SUFFIX), MMUSUFFIX)(env, addr, val, mmu_idx,
- GETPC_EXT());
+ GETRA_EXT());
}
#endif /* !defined(SOFTMMU_CODE_ACCESS) */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/7] exec: Reorganize the GETRA/GETPC macros
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 1/7] exec: Reorganize the GETRA/GETPC macros Richard Henderson
@ 2013-08-30 16:55 ` Aurelien Jarno
0 siblings, 0 replies; 20+ messages in thread
From: Aurelien Jarno @ 2013-08-30 16:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Thu, Aug 29, 2013 at 03:05:55PM -0700, Richard Henderson wrote:
> Always define GETRA; use __builtin_extract_return_addr, rather than
> having a special case for s390. Split GETPC_ADJ out of GETPC; use 2
> universally, rather than having a special case for arm.
>
> Rename GETPC_LDST to GETRA_LDST to indicate that it does not
> contain the GETPC_ADJ value. Likewise with GETPC_EXT to GETRA_EXT.
>
> Perform the GETPC_ADJ adjustment inside helper_ret_ld/st. This will
> allow backends to pass along the "true" return address rather than
> the massaged GETPC value. In the meantime, double application of
> GETPC_ADJ does not hurt, since the call insn in all ISAs is at least
> 4 bytes long.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> include/exec/exec-all.h | 84 +++++++++++++++++++----------------------
> include/exec/softmmu_template.h | 24 ++++++++----
> 2 files changed, 56 insertions(+), 52 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index ffb69a4..6f71a4f 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -295,47 +295,42 @@ static inline void tb_add_jump(TranslationBlock *tb, int n,
> }
> }
>
> -/* The return address may point to the start of the next instruction.
> - Subtracting one gets us the call instruction itself. */
> +/* GETRA is the true target of the return instruction that we'll execute,
> + defined here for simplicity of defining the follow-up macros. */
> #if defined(CONFIG_TCG_INTERPRETER)
> extern uintptr_t tci_tb_ptr;
> -# define GETPC() tci_tb_ptr
> -#elif defined(__s390__) && !defined(__s390x__)
> -# define GETPC() \
> - (((uintptr_t)__builtin_return_address(0) & 0x7fffffffUL) - 1)
> -#elif defined(__arm__)
> -/* Thumb return addresses have the low bit set, so we need to subtract two.
> - This is still safe in ARM mode because instructions are 4 bytes. */
> -# define GETPC() ((uintptr_t)__builtin_return_address(0) - 2)
> +# define GETRA() tci_tb_ptr
> +#else
> +# define GETRA() \
> + ((uintptr_t)__builtin_extract_return_addr(__builtin_return_address(0)))
> +#endif
> +
> +/* The true return address will often point to a host insn that is part of
> + the next translated guest insn. Adjust the address backward to point to
> + the middle of the call insn. Subtracting one would do the job except for
> + several compressed mode architectures (arm, mips) which set the low bit
> + to indicate the compressed mode; subtracting two works around that. It
> + is also the case that there are no host isas that contain a call insn
> + smaller than 4 bytes, so we don't worry about special-casing this. */
> +#if defined(CONFIG_TCG_INTERPRETER)
> +# define GETPC_ADJ 0
> #else
> -# define GETPC() ((uintptr_t)__builtin_return_address(0) - 1)
> +# define GETPC_ADJ 2
> #endif
>
> +#define GETPC() (GETRA() - GETPC_ADJ)
> +
> +/* The LDST optimizations splits code generation into fast and slow path.
> + In some implementations, we pass the "logical" return address manually;
> + in others, we must infer the logical return from the true return. */
> #if defined(CONFIG_QEMU_LDST_OPTIMIZATION) && defined(CONFIG_SOFTMMU)
> -/* qemu_ld/st optimization split code generation to fast and slow path, thus,
> - it needs special handling for an MMU helper which is called from the slow
> - path, to get the fast path's pc without any additional argument.
> - It uses a tricky solution which embeds the fast path pc into the slow path.
> -
> - Code flow in slow path:
> - (1) pre-process
> - (2) call MMU helper
> - (3) jump to (5)
> - (4) fast path information (implementation specific)
> - (5) post-process (e.g. stack adjust)
> - (6) jump to corresponding code of the next of fast path
> - */
> -# if defined(__i386__) || defined(__x86_64__)
> -# define GETPC_EXT() GETPC()
> -# elif defined (_ARCH_PPC) && !defined (_ARCH_PPC64)
> -# define GETRA() ((uintptr_t)__builtin_return_address(0))
> -# define GETPC_LDST() ((uintptr_t) ((*(int32_t *)(GETRA() - 4)) - 1))
> +# if defined (_ARCH_PPC) && !defined (_ARCH_PPC64)
> +# define GETRA_LDST(RA) (*(int32_t *)((RA) - 4))
> # elif defined(__arm__)
> /* We define two insns between the return address and the branch back to
> straight-line. Find and decode that branch insn. */
> -# define GETRA() ((uintptr_t)__builtin_return_address(0))
> -# define GETPC_LDST() tcg_getpc_ldst(GETRA())
> -static inline uintptr_t tcg_getpc_ldst(uintptr_t ra)
> +# define GETRA_LDST(RA) tcg_getra_ldst(RA)
> +static inline uintptr_t tcg_getra_ldst(uintptr_t ra)
> {
> int32_t b;
> ra += 8; /* skip the two insns */
> @@ -343,33 +338,32 @@ static inline uintptr_t tcg_getpc_ldst(uintptr_t ra)
> b = (b << 8) >> (8 - 2); /* extract the displacement */
> ra += 8; /* branches are relative to pc+8 */
> ra += b; /* apply the displacement */
> - ra -= 4; /* return a pointer into the current opcode,
> - not the start of the next opcode */
> return ra;
> }
> # elif defined(__aarch64__)
> -# define GETRA() ((uintptr_t)__builtin_return_address(0))
> -# define GETPC_LDST() tcg_getpc_ldst(GETRA())
> -static inline uintptr_t tcg_getpc_ldst(uintptr_t ra)
> +# define GETRA_LDST(RA) tcg_getra_ldst(RA)
> +static inline uintptr_t tcg_getra_ldst(uintptr_t ra)
> {
> int32_t b;
> ra += 4; /* skip one instruction */
> b = *(int32_t *)ra; /* load the branch insn */
> b = (b << 6) >> (6 - 2); /* extract the displacement */
> ra += b; /* apply the displacement */
> - ra -= 4; /* return a pointer into the current opcode,
> - not the start of the next opcode */
> return ra;
> }
> -# else
> -# error "CONFIG_QEMU_LDST_OPTIMIZATION needs GETPC_LDST() implementation!"
> # endif
> +#endif /* CONFIG_QEMU_LDST_OPTIMIZATION */
> +
> +/* ??? Delete these once they are no longer used. */
> bool is_tcg_gen_code(uintptr_t pc_ptr);
> -# ifndef GETPC_EXT
> -# define GETPC_EXT() (is_tcg_gen_code(GETRA()) ? GETPC_LDST() : GETPC())
> -# endif
> +#ifdef GETRA_LDST
> +# define GETRA_EXT() tcg_getra_ext(GETRA())
> +static inline uintptr_t tcg_getra_ext(uintptr_t ra)
> +{
> + return is_tcg_gen_code(ra) ? GETRA_LDST(ra) : ra;
> +}
> #else
> -# define GETPC_EXT() GETPC()
> +# define GETRA_EXT() GETRA()
> #endif
>
> #if !defined(CONFIG_USER_ONLY)
> diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
> index eaca9e1..2fc6ea3 100644
> --- a/include/exec/softmmu_template.h
> +++ b/include/exec/softmmu_template.h
> @@ -86,6 +86,9 @@ glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env,
> target_ulong tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ;
> uintptr_t haddr;
>
> + /* Adjust the given return address. */
> + retaddr -= GETPC_ADJ;
> +
> /* If the TLB entry is for a different page, reload and try again. */
> if ((addr & TARGET_PAGE_MASK)
> != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) {
> @@ -121,10 +124,12 @@ glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env,
> #endif
> addr1 = addr & ~(DATA_SIZE - 1);
> addr2 = addr1 + DATA_SIZE;
> - res1 = glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(env, addr1,
> - mmu_idx, retaddr);
> - res2 = glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(env, addr2,
> - mmu_idx, retaddr);
> + /* Note the adjustment at the beginning of the function.
> + Undo that for the recursion. */
> + res1 = glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)
> + (env, addr1, mmu_idx, retaddr + GETPC_ADJ);
> + res2 = glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)
> + (env, addr2, mmu_idx, retaddr + GETPC_ADJ);
> shift = (addr & (DATA_SIZE - 1)) * 8;
> #ifdef TARGET_WORDS_BIGENDIAN
> res = (res1 << shift) | (res2 >> ((DATA_SIZE * 8) - shift));
> @@ -150,7 +155,7 @@ glue(glue(helper_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
> int mmu_idx)
> {
> return glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(env, addr, mmu_idx,
> - GETPC_EXT());
> + GETRA_EXT());
> }
>
> #ifndef SOFTMMU_CODE_ACCESS
> @@ -182,6 +187,9 @@ glue(glue(helper_ret_st, SUFFIX), MMUSUFFIX)(CPUArchState *env,
> target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write;
> uintptr_t haddr;
>
> + /* Adjust the given return address. */
> + retaddr -= GETPC_ADJ;
> +
> /* If the TLB entry is for a different page, reload and try again. */
> if ((addr & TARGET_PAGE_MASK)
> != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) {
> @@ -223,8 +231,10 @@ glue(glue(helper_ret_st, SUFFIX), MMUSUFFIX)(CPUArchState *env,
> #else
> uint8_t val8 = val >> (i * 8);
> #endif
> + /* Note the adjustment at the beginning of the function.
> + Undo that for the recursion. */
> glue(helper_ret_stb, MMUSUFFIX)(env, addr + i, val8,
> - mmu_idx, retaddr);
> + mmu_idx, retaddr + GETPC_ADJ);
> }
> return;
> }
> @@ -245,7 +255,7 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
> DATA_TYPE val, int mmu_idx)
> {
> glue(glue(helper_ret_st, SUFFIX), MMUSUFFIX)(env, addr, val, mmu_idx,
> - GETPC_EXT());
> + GETRA_EXT());
> }
>
> #endif /* !defined(SOFTMMU_CODE_ACCESS) */
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 2/7] tcg-i386: Don't perform GETPC adjustment in TCG code
2013-08-29 22:05 [Qemu-devel] [PATCH v2 0/7] Further tcg ldst improvements Richard Henderson
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 1/7] exec: Reorganize the GETRA/GETPC macros Richard Henderson
@ 2013-08-29 22:05 ` Richard Henderson
2013-08-30 16:55 ` Aurelien Jarno
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 3/7] exec: Rename USUFFIX to LSUFFIX Richard Henderson
` (4 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2013-08-29 22:05 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien
Since we now perform it inside the helper, no need to do it here.
This also lets us perform a tail-call from the store slow path to
the helper.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
tcg/i386/tcg-target.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index 12a7ca3..84f17fe 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -1470,12 +1470,6 @@ static void add_qemu_ldst_label(TCGContext *s,
}
}
-/* See the GETPC definition in include/exec/exec-all.h. */
-static inline uintptr_t do_getpc(uint8_t *raddr)
-{
- return (uintptr_t)raddr - 1;
-}
-
/*
* Generate code for the slow path for a load at the end of block
*/
@@ -1509,14 +1503,14 @@ static void tcg_out_qemu_ld_slow_path(TCGContext *s, TCGLabelQemuLdst *l)
tcg_out_sti(s, TCG_TYPE_I32, TCG_REG_ESP, ofs, l->mem_index);
ofs += 4;
- tcg_out_sti(s, TCG_TYPE_I32, TCG_REG_ESP, ofs, do_getpc(l->raddr));
+ tcg_out_sti(s, TCG_TYPE_I32, TCG_REG_ESP, ofs, (uintptr_t)l->raddr);
} else {
tcg_out_mov(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[0], TCG_AREG0);
/* The second argument is already loaded with addrlo. */
tcg_out_movi(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[2],
l->mem_index);
tcg_out_movi(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[3],
- do_getpc(l->raddr));
+ (uintptr_t)l->raddr);
}
tcg_out_calli(s, (tcg_target_long)qemu_ld_helpers[s_bits]);
@@ -1571,6 +1565,7 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, TCGLabelQemuLdst *l)
int opc = l->opc;
int s_bits = opc & 3;
uint8_t **label_ptr = &l->label_ptr[0];
+ TCGReg retaddr;
/* resolve label address */
*(uint32_t *)label_ptr[0] = (uint32_t)(s->code_ptr - label_ptr[0] - 4);
@@ -1603,10 +1598,10 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, TCGLabelQemuLdst *l)
tcg_out_sti(s, TCG_TYPE_I32, TCG_REG_ESP, ofs, l->mem_index);
ofs += 4;
- tcg_out_sti(s, TCG_TYPE_I32, TCG_REG_ESP, ofs, do_getpc(l->raddr));
+ retaddr = TCG_REG_EAX;
+ tcg_out_movi(s, TCG_TYPE_I32, retaddr, (uintptr_t)l->raddr);
+ tcg_out_st(s, TCG_TYPE_I32, retaddr, TCG_REG_ESP, ofs);
} else {
- uintptr_t pc;
-
tcg_out_mov(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[0], TCG_AREG0);
/* The second argument is already loaded with addrlo. */
tcg_out_mov(s, (opc == 3 ? TCG_TYPE_I64 : TCG_TYPE_I32),
@@ -1614,20 +1609,19 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, TCGLabelQemuLdst *l)
tcg_out_movi(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[3],
l->mem_index);
- pc = do_getpc(l->raddr);
if (ARRAY_SIZE(tcg_target_call_iarg_regs) > 4) {
- tcg_out_movi(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[4], pc);
- } else if (pc == (int32_t)pc) {
- tcg_out_sti(s, TCG_TYPE_PTR, TCG_REG_ESP, 0, pc);
+ retaddr = tcg_target_call_iarg_regs[4];
+ tcg_out_movi(s, TCG_TYPE_PTR, retaddr, (uintptr_t)l->raddr);
} else {
- tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_RAX, pc);
- tcg_out_st(s, TCG_TYPE_PTR, TCG_REG_RAX, TCG_REG_ESP, 0);
+ retaddr = TCG_REG_RAX;
+ tcg_out_movi(s, TCG_TYPE_PTR, retaddr, (uintptr_t)l->raddr);
+ tcg_out_st(s, TCG_TYPE_PTR, retaddr, TCG_REG_ESP, 0);
}
}
- tcg_out_calli(s, (tcg_target_long)qemu_st_helpers[s_bits]);
-
- tcg_out_jmp(s, (tcg_target_long)l->raddr);
+ /* "Tail call" to the helper, with the return address back inline. */
+ tcg_out_push(s, retaddr);
+ tcg_out_jmp(s, (tcg_target_long)qemu_st_helpers[s_bits]);
}
/*
--
1.8.1.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/7] tcg-i386: Don't perform GETPC adjustment in TCG code
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 2/7] tcg-i386: Don't perform GETPC adjustment in TCG code Richard Henderson
@ 2013-08-30 16:55 ` Aurelien Jarno
0 siblings, 0 replies; 20+ messages in thread
From: Aurelien Jarno @ 2013-08-30 16:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Thu, Aug 29, 2013 at 03:05:56PM -0700, Richard Henderson wrote:
> Since we now perform it inside the helper, no need to do it here.
> This also lets us perform a tail-call from the store slow path to
> the helper.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> tcg/i386/tcg-target.c | 34 ++++++++++++++--------------------
> 1 file changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index 12a7ca3..84f17fe 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -1470,12 +1470,6 @@ static void add_qemu_ldst_label(TCGContext *s,
> }
> }
>
> -/* See the GETPC definition in include/exec/exec-all.h. */
> -static inline uintptr_t do_getpc(uint8_t *raddr)
> -{
> - return (uintptr_t)raddr - 1;
> -}
> -
> /*
> * Generate code for the slow path for a load at the end of block
> */
> @@ -1509,14 +1503,14 @@ static void tcg_out_qemu_ld_slow_path(TCGContext *s, TCGLabelQemuLdst *l)
> tcg_out_sti(s, TCG_TYPE_I32, TCG_REG_ESP, ofs, l->mem_index);
> ofs += 4;
>
> - tcg_out_sti(s, TCG_TYPE_I32, TCG_REG_ESP, ofs, do_getpc(l->raddr));
> + tcg_out_sti(s, TCG_TYPE_I32, TCG_REG_ESP, ofs, (uintptr_t)l->raddr);
> } else {
> tcg_out_mov(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[0], TCG_AREG0);
> /* The second argument is already loaded with addrlo. */
> tcg_out_movi(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[2],
> l->mem_index);
> tcg_out_movi(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[3],
> - do_getpc(l->raddr));
> + (uintptr_t)l->raddr);
> }
>
> tcg_out_calli(s, (tcg_target_long)qemu_ld_helpers[s_bits]);
> @@ -1571,6 +1565,7 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, TCGLabelQemuLdst *l)
> int opc = l->opc;
> int s_bits = opc & 3;
> uint8_t **label_ptr = &l->label_ptr[0];
> + TCGReg retaddr;
>
> /* resolve label address */
> *(uint32_t *)label_ptr[0] = (uint32_t)(s->code_ptr - label_ptr[0] - 4);
> @@ -1603,10 +1598,10 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, TCGLabelQemuLdst *l)
> tcg_out_sti(s, TCG_TYPE_I32, TCG_REG_ESP, ofs, l->mem_index);
> ofs += 4;
>
> - tcg_out_sti(s, TCG_TYPE_I32, TCG_REG_ESP, ofs, do_getpc(l->raddr));
> + retaddr = TCG_REG_EAX;
> + tcg_out_movi(s, TCG_TYPE_I32, retaddr, (uintptr_t)l->raddr);
> + tcg_out_st(s, TCG_TYPE_I32, retaddr, TCG_REG_ESP, ofs);
> } else {
> - uintptr_t pc;
> -
> tcg_out_mov(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[0], TCG_AREG0);
> /* The second argument is already loaded with addrlo. */
> tcg_out_mov(s, (opc == 3 ? TCG_TYPE_I64 : TCG_TYPE_I32),
> @@ -1614,20 +1609,19 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, TCGLabelQemuLdst *l)
> tcg_out_movi(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[3],
> l->mem_index);
>
> - pc = do_getpc(l->raddr);
> if (ARRAY_SIZE(tcg_target_call_iarg_regs) > 4) {
> - tcg_out_movi(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[4], pc);
> - } else if (pc == (int32_t)pc) {
> - tcg_out_sti(s, TCG_TYPE_PTR, TCG_REG_ESP, 0, pc);
> + retaddr = tcg_target_call_iarg_regs[4];
> + tcg_out_movi(s, TCG_TYPE_PTR, retaddr, (uintptr_t)l->raddr);
> } else {
> - tcg_out_movi(s, TCG_TYPE_PTR, TCG_REG_RAX, pc);
> - tcg_out_st(s, TCG_TYPE_PTR, TCG_REG_RAX, TCG_REG_ESP, 0);
> + retaddr = TCG_REG_RAX;
> + tcg_out_movi(s, TCG_TYPE_PTR, retaddr, (uintptr_t)l->raddr);
> + tcg_out_st(s, TCG_TYPE_PTR, retaddr, TCG_REG_ESP, 0);
> }
> }
>
> - tcg_out_calli(s, (tcg_target_long)qemu_st_helpers[s_bits]);
> -
> - tcg_out_jmp(s, (tcg_target_long)l->raddr);
> + /* "Tail call" to the helper, with the return address back inline. */
> + tcg_out_push(s, retaddr);
> + tcg_out_jmp(s, (tcg_target_long)qemu_st_helpers[s_bits]);
> }
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 3/7] exec: Rename USUFFIX to LSUFFIX
2013-08-29 22:05 [Qemu-devel] [PATCH v2 0/7] Further tcg ldst improvements Richard Henderson
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 1/7] exec: Reorganize the GETRA/GETPC macros Richard Henderson
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 2/7] tcg-i386: Don't perform GETPC adjustment in TCG code Richard Henderson
@ 2013-08-29 22:05 ` Richard Henderson
2013-08-30 16:55 ` Aurelien Jarno
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 4/7] target: Include softmmu_exec.h where forgotten Richard Henderson
` (3 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2013-08-29 22:05 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien
In a following patch, there will be confusion between multiple "unsigned"
suffixes; rename this one so as to imply "load".
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
include/exec/softmmu_template.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
index 2fc6ea3..f9922e2 100644
--- a/include/exec/softmmu_template.h
+++ b/include/exec/softmmu_template.h
@@ -28,19 +28,19 @@
#if DATA_SIZE == 8
#define SUFFIX q
-#define USUFFIX q
+#define LSUFFIX q
#define DATA_TYPE uint64_t
#elif DATA_SIZE == 4
#define SUFFIX l
-#define USUFFIX l
+#define LSUFFIX l
#define DATA_TYPE uint32_t
#elif DATA_SIZE == 2
#define SUFFIX w
-#define USUFFIX uw
+#define LSUFFIX uw
#define DATA_TYPE uint16_t
#elif DATA_SIZE == 1
#define SUFFIX b
-#define USUFFIX ub
+#define LSUFFIX ub
#define DATA_TYPE uint8_t
#else
#error unsupported data size
@@ -147,7 +147,7 @@ glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env,
#endif
haddr = addr + env->tlb_table[mmu_idx][index].addend;
- return glue(glue(ld, USUFFIX), _raw)((uint8_t *)haddr);
+ return glue(glue(ld, LSUFFIX), _raw)((uint8_t *)haddr);
}
DATA_TYPE
@@ -264,6 +264,6 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
#undef SHIFT
#undef DATA_TYPE
#undef SUFFIX
-#undef USUFFIX
+#undef LSUFFIX
#undef DATA_SIZE
#undef ADDR_READ
--
1.8.1.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/7] exec: Rename USUFFIX to LSUFFIX
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 3/7] exec: Rename USUFFIX to LSUFFIX Richard Henderson
@ 2013-08-30 16:55 ` Aurelien Jarno
0 siblings, 0 replies; 20+ messages in thread
From: Aurelien Jarno @ 2013-08-30 16:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Thu, Aug 29, 2013 at 03:05:57PM -0700, Richard Henderson wrote:
> In a following patch, there will be confusion between multiple "unsigned"
> suffixes; rename this one so as to imply "load".
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> include/exec/softmmu_template.h | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
> index 2fc6ea3..f9922e2 100644
> --- a/include/exec/softmmu_template.h
> +++ b/include/exec/softmmu_template.h
> @@ -28,19 +28,19 @@
>
> #if DATA_SIZE == 8
> #define SUFFIX q
> -#define USUFFIX q
> +#define LSUFFIX q
> #define DATA_TYPE uint64_t
> #elif DATA_SIZE == 4
> #define SUFFIX l
> -#define USUFFIX l
> +#define LSUFFIX l
> #define DATA_TYPE uint32_t
> #elif DATA_SIZE == 2
> #define SUFFIX w
> -#define USUFFIX uw
> +#define LSUFFIX uw
> #define DATA_TYPE uint16_t
> #elif DATA_SIZE == 1
> #define SUFFIX b
> -#define USUFFIX ub
> +#define LSUFFIX ub
> #define DATA_TYPE uint8_t
> #else
> #error unsupported data size
> @@ -147,7 +147,7 @@ glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env,
> #endif
>
> haddr = addr + env->tlb_table[mmu_idx][index].addend;
> - return glue(glue(ld, USUFFIX), _raw)((uint8_t *)haddr);
> + return glue(glue(ld, LSUFFIX), _raw)((uint8_t *)haddr);
> }
>
> DATA_TYPE
> @@ -264,6 +264,6 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
> #undef SHIFT
> #undef DATA_TYPE
> #undef SUFFIX
> -#undef USUFFIX
> +#undef LSUFFIX
> #undef DATA_SIZE
> #undef ADDR_READ
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 4/7] target: Include softmmu_exec.h where forgotten
2013-08-29 22:05 [Qemu-devel] [PATCH v2 0/7] Further tcg ldst improvements Richard Henderson
` (2 preceding siblings ...)
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 3/7] exec: Rename USUFFIX to LSUFFIX Richard Henderson
@ 2013-08-29 22:05 ` Richard Henderson
2013-08-30 16:55 ` Aurelien Jarno
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 5/7] exec: Split softmmu_defs.h Richard Henderson
` (2 subsequent siblings)
6 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2013-08-29 22:05 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien
Several targets forgot to include softmmu_exec.h, which would
break them with a header cleanup to follow.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
target-lm32/op_helper.c | 2 ++
target-moxie/helper.c | 1 +
target-ppc/mmu_helper.c | 2 ++
target-unicore32/op_helper.c | 2 ++
target-xtensa/op_helper.c | 1 +
5 files changed, 8 insertions(+)
diff --git a/target-lm32/op_helper.c b/target-lm32/op_helper.c
index 2dab9f2..8f5ef55 100644
--- a/target-lm32/op_helper.c
+++ b/target-lm32/op_helper.c
@@ -6,6 +6,8 @@
#include "hw/lm32/lm32_pic.h"
#include "hw/char/lm32_juart.h"
+#include "exec/softmmu_exec.h"
+
#if !defined(CONFIG_USER_ONLY)
#define MMUSUFFIX _mmu
#define SHIFT 0
diff --git a/target-moxie/helper.c b/target-moxie/helper.c
index b12e4ff..7859102 100644
--- a/target-moxie/helper.c
+++ b/target-moxie/helper.c
@@ -25,6 +25,7 @@
#include "cpu.h"
#include "mmu.h"
#include "exec/exec-all.h"
+#include "exec/softmmu_exec.h"
#include "qemu/host-utils.h"
#include "helper.h"
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index 5dd4e05..44f04e5 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -2871,6 +2871,8 @@ void helper_booke206_tlbflush(CPUPPCState *env, uint32_t type)
/*****************************************************************************/
+#include "exec/softmmu_exec.h"
+
#define MMUSUFFIX _mmu
#define SHIFT 0
diff --git a/target-unicore32/op_helper.c b/target-unicore32/op_helper.c
index 6443ffe..4f9f41e 100644
--- a/target-unicore32/op_helper.c
+++ b/target-unicore32/op_helper.c
@@ -239,6 +239,8 @@ uint32_t HELPER(ror_cc)(CPUUniCore32State *env, uint32_t x, uint32_t i)
}
#ifndef CONFIG_USER_ONLY
+#include "exec/softmmu_exec.h"
+
#define MMUSUFFIX _mmu
#define SHIFT 0
diff --git a/target-xtensa/op_helper.c b/target-xtensa/op_helper.c
index 01123af..cf97025 100644
--- a/target-xtensa/op_helper.c
+++ b/target-xtensa/op_helper.c
@@ -28,6 +28,7 @@
#include "cpu.h"
#include "helper.h"
#include "qemu/host-utils.h"
+#include "exec/softmmu_exec.h"
static void do_unaligned_access(CPUXtensaState *env,
target_ulong addr, int is_write, int is_user, uintptr_t retaddr);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 4/7] target: Include softmmu_exec.h where forgotten
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 4/7] target: Include softmmu_exec.h where forgotten Richard Henderson
@ 2013-08-30 16:55 ` Aurelien Jarno
0 siblings, 0 replies; 20+ messages in thread
From: Aurelien Jarno @ 2013-08-30 16:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Thu, Aug 29, 2013 at 03:05:58PM -0700, Richard Henderson wrote:
> Several targets forgot to include softmmu_exec.h, which would
> break them with a header cleanup to follow.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> target-lm32/op_helper.c | 2 ++
> target-moxie/helper.c | 1 +
> target-ppc/mmu_helper.c | 2 ++
> target-unicore32/op_helper.c | 2 ++
> target-xtensa/op_helper.c | 1 +
> 5 files changed, 8 insertions(+)
>
> diff --git a/target-lm32/op_helper.c b/target-lm32/op_helper.c
> index 2dab9f2..8f5ef55 100644
> --- a/target-lm32/op_helper.c
> +++ b/target-lm32/op_helper.c
> @@ -6,6 +6,8 @@
> #include "hw/lm32/lm32_pic.h"
> #include "hw/char/lm32_juart.h"
>
> +#include "exec/softmmu_exec.h"
> +
> #if !defined(CONFIG_USER_ONLY)
> #define MMUSUFFIX _mmu
> #define SHIFT 0
> diff --git a/target-moxie/helper.c b/target-moxie/helper.c
> index b12e4ff..7859102 100644
> --- a/target-moxie/helper.c
> +++ b/target-moxie/helper.c
> @@ -25,6 +25,7 @@
> #include "cpu.h"
> #include "mmu.h"
> #include "exec/exec-all.h"
> +#include "exec/softmmu_exec.h"
> #include "qemu/host-utils.h"
> #include "helper.h"
>
> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
> index 5dd4e05..44f04e5 100644
> --- a/target-ppc/mmu_helper.c
> +++ b/target-ppc/mmu_helper.c
> @@ -2871,6 +2871,8 @@ void helper_booke206_tlbflush(CPUPPCState *env, uint32_t type)
>
> /*****************************************************************************/
>
> +#include "exec/softmmu_exec.h"
> +
> #define MMUSUFFIX _mmu
>
> #define SHIFT 0
> diff --git a/target-unicore32/op_helper.c b/target-unicore32/op_helper.c
> index 6443ffe..4f9f41e 100644
> --- a/target-unicore32/op_helper.c
> +++ b/target-unicore32/op_helper.c
> @@ -239,6 +239,8 @@ uint32_t HELPER(ror_cc)(CPUUniCore32State *env, uint32_t x, uint32_t i)
> }
>
> #ifndef CONFIG_USER_ONLY
> +#include "exec/softmmu_exec.h"
> +
> #define MMUSUFFIX _mmu
>
> #define SHIFT 0
> diff --git a/target-xtensa/op_helper.c b/target-xtensa/op_helper.c
> index 01123af..cf97025 100644
> --- a/target-xtensa/op_helper.c
> +++ b/target-xtensa/op_helper.c
> @@ -28,6 +28,7 @@
> #include "cpu.h"
> #include "helper.h"
> #include "qemu/host-utils.h"
> +#include "exec/softmmu_exec.h"
>
> static void do_unaligned_access(CPUXtensaState *env,
> target_ulong addr, int is_write, int is_user, uintptr_t retaddr);
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 5/7] exec: Split softmmu_defs.h
2013-08-29 22:05 [Qemu-devel] [PATCH v2 0/7] Further tcg ldst improvements Richard Henderson
` (3 preceding siblings ...)
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 4/7] target: Include softmmu_exec.h where forgotten Richard Henderson
@ 2013-08-29 22:05 ` Richard Henderson
2013-08-30 16:55 ` Aurelien Jarno
2013-08-29 22:06 ` [Qemu-devel] [PATCH v2 6/7] tcg: Introduce zero and sign-extended versions of load helpers Richard Henderson
2013-08-29 22:06 ` [Qemu-devel] [PATCH v2 7/7] tcg-i386: Make use of zero-extended memory helper routines Richard Henderson
6 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2013-08-29 22:05 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien
The _cmmu helpers can be moved to exec-all.h. The helpers that are
used from TCG will shortly need access to tcg_target_long so move
their declarations into tcg.h.
This requires minor include adjustments to all TCG backends.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
include/exec/exec-all.h | 5 ++++-
include/exec/softmmu_defs.h | 49 ---------------------------------------------
include/exec/softmmu_exec.h | 3 ++-
tcg/aarch64/tcg-target.c | 2 --
tcg/arm/tcg-target.c | 2 --
tcg/hppa/tcg-target.c | 2 --
tcg/i386/tcg-target.c | 3 ---
tcg/ia64/tcg-target.c | 3 ---
tcg/mips/tcg-target.c | 3 ---
tcg/ppc/tcg-target.c | 2 --
tcg/ppc64/tcg-target.c | 3 ---
tcg/s390/tcg-target.c | 3 ---
tcg/sparc/tcg-target.c | 2 --
tcg/tcg.h | 43 +++++++++++++++++++++++++++++++++++++++
14 files changed, 49 insertions(+), 76 deletions(-)
delete mode 100644 include/exec/softmmu_defs.h
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 6f71a4f..beb4149 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -377,7 +377,10 @@ bool io_mem_write(struct MemoryRegion *mr, hwaddr addr,
void tlb_fill(CPUArchState *env1, target_ulong addr, int is_write, int mmu_idx,
uintptr_t retaddr);
-#include "exec/softmmu_defs.h"
+uint8_t helper_ldb_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
+uint16_t helper_ldw_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
+uint32_t helper_ldl_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
+uint64_t helper_ldq_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
#define ACCESS_TYPE (NB_MMU_MODES + 1)
#define MEMSUFFIX _code
diff --git a/include/exec/softmmu_defs.h b/include/exec/softmmu_defs.h
deleted file mode 100644
index e55e717..0000000
--- a/include/exec/softmmu_defs.h
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- * Software MMU support
- *
- * Declare helpers used by TCG for qemu_ld/st ops.
- *
- * Used by softmmu_exec.h, TCG targets and exec-all.h.
- *
- */
-#ifndef SOFTMMU_DEFS_H
-#define SOFTMMU_DEFS_H
-
-uint8_t helper_ret_ldb_mmu(CPUArchState *env, target_ulong addr,
- int mmu_idx, uintptr_t retaddr);
-uint16_t helper_ret_ldw_mmu(CPUArchState *env, target_ulong addr,
- int mmu_idx, uintptr_t retaddr);
-uint32_t helper_ret_ldl_mmu(CPUArchState *env, target_ulong addr,
- int mmu_idx, uintptr_t retaddr);
-uint64_t helper_ret_ldq_mmu(CPUArchState *env, target_ulong addr,
- int mmu_idx, uintptr_t retaddr);
-
-void helper_ret_stb_mmu(CPUArchState *env, target_ulong addr, uint8_t val,
- int mmu_idx, uintptr_t retaddr);
-void helper_ret_stw_mmu(CPUArchState *env, target_ulong addr, uint16_t val,
- int mmu_idx, uintptr_t retaddr);
-void helper_ret_stl_mmu(CPUArchState *env, target_ulong addr, uint32_t val,
- int mmu_idx, uintptr_t retaddr);
-void helper_ret_stq_mmu(CPUArchState *env, target_ulong addr, uint64_t val,
- int mmu_idx, uintptr_t retaddr);
-
-uint8_t helper_ldb_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-uint16_t helper_ldw_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-uint32_t helper_ldl_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-uint64_t helper_ldq_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-
-void helper_stb_mmu(CPUArchState *env, target_ulong addr,
- uint8_t val, int mmu_idx);
-void helper_stw_mmu(CPUArchState *env, target_ulong addr,
- uint16_t val, int mmu_idx);
-void helper_stl_mmu(CPUArchState *env, target_ulong addr,
- uint32_t val, int mmu_idx);
-void helper_stq_mmu(CPUArchState *env, target_ulong addr,
- uint64_t val, int mmu_idx);
-
-uint8_t helper_ldb_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-uint16_t helper_ldw_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-uint32_t helper_ldl_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-uint64_t helper_ldq_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
-
-#endif /* SOFTMMU_DEFS_H */
diff --git a/include/exec/softmmu_exec.h b/include/exec/softmmu_exec.h
index 3e4e886..6fde154 100644
--- a/include/exec/softmmu_exec.h
+++ b/include/exec/softmmu_exec.h
@@ -19,7 +19,8 @@
#define ldul_executive ldl_executive
#define ldul_supervisor ldl_supervisor
-#include "exec/softmmu_defs.h"
+/* The memory helpers for tcg-generated code need tcg_target_long etc. */
+#include "tcg.h"
#define ACCESS_TYPE 0
#define MEMSUFFIX MMU_MODE0_SUFFIX
diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
index 41a17f8..55ff700 100644
--- a/tcg/aarch64/tcg-target.c
+++ b/tcg/aarch64/tcg-target.c
@@ -778,8 +778,6 @@ static inline void tcg_out_nop(TCGContext *s)
}
#ifdef CONFIG_SOFTMMU
-#include "exec/softmmu_defs.h"
-
/* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
int mmu_idx) */
static const void * const qemu_ld_helpers[4] = {
diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
index 6c4854d..6d084b3 100644
--- a/tcg/arm/tcg-target.c
+++ b/tcg/arm/tcg-target.c
@@ -1058,8 +1058,6 @@ static inline void tcg_out_goto_label(TCGContext *s, int cond, int label_index)
#ifdef CONFIG_SOFTMMU
-#include "exec/softmmu_defs.h"
-
/* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
int mmu_idx) */
static const void * const qemu_ld_helpers[4] = {
diff --git a/tcg/hppa/tcg-target.c b/tcg/hppa/tcg-target.c
index 68f77ba..bac1f12 100644
--- a/tcg/hppa/tcg-target.c
+++ b/tcg/hppa/tcg-target.c
@@ -906,8 +906,6 @@ static void tcg_out_movcond(TCGContext *s, int cond, TCGArg ret,
}
#if defined(CONFIG_SOFTMMU)
-#include "exec/softmmu_defs.h"
-
/* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
int mmu_idx) */
static const void * const qemu_ld_helpers[4] = {
diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index 84f17fe..4c98cc4 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -1022,9 +1022,6 @@ static void tcg_out_jmp(TCGContext *s, tcg_target_long dest)
}
#if defined(CONFIG_SOFTMMU)
-
-#include "exec/softmmu_defs.h"
-
/* helper signature: helper_ret_ld_mmu(CPUState *env, target_ulong addr,
* int mmu_idx, uintptr_t ra)
*/
diff --git a/tcg/ia64/tcg-target.c b/tcg/ia64/tcg-target.c
index 2373d9e..56a1b3a 100644
--- a/tcg/ia64/tcg-target.c
+++ b/tcg/ia64/tcg-target.c
@@ -1490,9 +1490,6 @@ static inline void tcg_out_movcond(TCGContext *s, TCGCond cond, TCGArg ret,
}
#if defined(CONFIG_SOFTMMU)
-
-#include "exec/softmmu_defs.h"
-
/* Load and compare a TLB entry, and return the result in (p6, p7).
R2 is loaded with the address of the addend TLB entry.
R57 is loaded with the address, zero extented on 32-bit targets. */
diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
index 793532e..a750d42 100644
--- a/tcg/mips/tcg-target.c
+++ b/tcg/mips/tcg-target.c
@@ -919,9 +919,6 @@ static void tcg_out_setcond2(TCGContext *s, TCGCond cond, TCGReg ret,
}
#if defined(CONFIG_SOFTMMU)
-
-#include "exec/softmmu_defs.h"
-
/* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
int mmu_idx) */
static const void * const qemu_ld_helpers[4] = {
diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
index 453ab6b..9a73d06 100644
--- a/tcg/ppc/tcg-target.c
+++ b/tcg/ppc/tcg-target.c
@@ -549,8 +549,6 @@ static void add_qemu_ldst_label (TCGContext *s,
label->label_ptr[0] = label_ptr;
}
-#include "exec/softmmu_defs.h"
-
/* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
int mmu_idx) */
static const void * const qemu_ld_helpers[4] = {
diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
index 0678de2..ec067e6 100644
--- a/tcg/ppc64/tcg-target.c
+++ b/tcg/ppc64/tcg-target.c
@@ -750,9 +750,6 @@ static void tcg_out_ldsta(TCGContext *s, TCGReg ret, TCGReg addr,
}
#if defined (CONFIG_SOFTMMU)
-
-#include "exec/softmmu_defs.h"
-
/* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
int mmu_idx) */
static const void * const qemu_ld_helpers[4] = {
diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.c
index f229f1c..5b556fe 100644
--- a/tcg/s390/tcg-target.c
+++ b/tcg/s390/tcg-target.c
@@ -315,9 +315,6 @@ static const uint8_t tcg_cond_to_ltr_cond[] = {
};
#ifdef CONFIG_SOFTMMU
-
-#include "exec/softmmu_defs.h"
-
/* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
int mmu_idx) */
static const void * const qemu_ld_helpers[4] = {
diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
index 5bfd29c..b1e3fc1 100644
--- a/tcg/sparc/tcg-target.c
+++ b/tcg/sparc/tcg-target.c
@@ -831,8 +831,6 @@ static void tcg_target_qemu_prologue(TCGContext *s)
#if defined(CONFIG_SOFTMMU)
-#include "exec/softmmu_defs.h"
-
/* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
int mmu_idx) */
static const void * const qemu_ld_helpers[4] = {
diff --git a/tcg/tcg.h b/tcg/tcg.h
index f3f9889..eeff6b7 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -21,6 +21,10 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
+
+#ifndef TCG_H
+#define TCG_H
+
#include "qemu-common.h"
/* Target word size (must be identical to pointer size). */
@@ -741,3 +745,42 @@ void tcg_register_jit(void *buf, size_t buf_size);
/* Generate TB finalization at the end of block */
void tcg_out_tb_finalize(TCGContext *s);
#endif
+
+/*
+ * Memory helpers that will be used by TCG generated code.
+ */
+#ifdef CONFIG_SOFTMMU
+uint8_t helper_ret_ldb_mmu(CPUArchState *env, target_ulong addr,
+ int mmu_idx, uintptr_t retaddr);
+uint16_t helper_ret_ldw_mmu(CPUArchState *env, target_ulong addr,
+ int mmu_idx, uintptr_t retaddr);
+uint32_t helper_ret_ldl_mmu(CPUArchState *env, target_ulong addr,
+ int mmu_idx, uintptr_t retaddr);
+uint64_t helper_ret_ldq_mmu(CPUArchState *env, target_ulong addr,
+ int mmu_idx, uintptr_t retaddr);
+
+void helper_ret_stb_mmu(CPUArchState *env, target_ulong addr, uint8_t val,
+ int mmu_idx, uintptr_t retaddr);
+void helper_ret_stw_mmu(CPUArchState *env, target_ulong addr, uint16_t val,
+ int mmu_idx, uintptr_t retaddr);
+void helper_ret_stl_mmu(CPUArchState *env, target_ulong addr, uint32_t val,
+ int mmu_idx, uintptr_t retaddr);
+void helper_ret_stq_mmu(CPUArchState *env, target_ulong addr, uint64_t val,
+ int mmu_idx, uintptr_t retaddr);
+
+uint8_t helper_ldb_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
+uint16_t helper_ldw_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
+uint32_t helper_ldl_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
+uint64_t helper_ldq_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
+
+void helper_stb_mmu(CPUArchState *env, target_ulong addr,
+ uint8_t val, int mmu_idx);
+void helper_stw_mmu(CPUArchState *env, target_ulong addr,
+ uint16_t val, int mmu_idx);
+void helper_stl_mmu(CPUArchState *env, target_ulong addr,
+ uint32_t val, int mmu_idx);
+void helper_stq_mmu(CPUArchState *env, target_ulong addr,
+ uint64_t val, int mmu_idx);
+#endif /* CONFIG_SOFTMMU */
+
+#endif /* TCG_H */
--
1.8.1.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 5/7] exec: Split softmmu_defs.h
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 5/7] exec: Split softmmu_defs.h Richard Henderson
@ 2013-08-30 16:55 ` Aurelien Jarno
0 siblings, 0 replies; 20+ messages in thread
From: Aurelien Jarno @ 2013-08-30 16:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Thu, Aug 29, 2013 at 03:05:59PM -0700, Richard Henderson wrote:
> The _cmmu helpers can be moved to exec-all.h. The helpers that are
> used from TCG will shortly need access to tcg_target_long so move
> their declarations into tcg.h.
>
> This requires minor include adjustments to all TCG backends.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> include/exec/exec-all.h | 5 ++++-
> include/exec/softmmu_defs.h | 49 ---------------------------------------------
> include/exec/softmmu_exec.h | 3 ++-
> tcg/aarch64/tcg-target.c | 2 --
> tcg/arm/tcg-target.c | 2 --
> tcg/hppa/tcg-target.c | 2 --
> tcg/i386/tcg-target.c | 3 ---
> tcg/ia64/tcg-target.c | 3 ---
> tcg/mips/tcg-target.c | 3 ---
> tcg/ppc/tcg-target.c | 2 --
> tcg/ppc64/tcg-target.c | 3 ---
> tcg/s390/tcg-target.c | 3 ---
> tcg/sparc/tcg-target.c | 2 --
> tcg/tcg.h | 43 +++++++++++++++++++++++++++++++++++++++
> 14 files changed, 49 insertions(+), 76 deletions(-)
> delete mode 100644 include/exec/softmmu_defs.h
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 6f71a4f..beb4149 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -377,7 +377,10 @@ bool io_mem_write(struct MemoryRegion *mr, hwaddr addr,
> void tlb_fill(CPUArchState *env1, target_ulong addr, int is_write, int mmu_idx,
> uintptr_t retaddr);
>
> -#include "exec/softmmu_defs.h"
> +uint8_t helper_ldb_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> +uint16_t helper_ldw_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> +uint32_t helper_ldl_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> +uint64_t helper_ldq_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
>
> #define ACCESS_TYPE (NB_MMU_MODES + 1)
> #define MEMSUFFIX _code
> diff --git a/include/exec/softmmu_defs.h b/include/exec/softmmu_defs.h
> deleted file mode 100644
> index e55e717..0000000
> --- a/include/exec/softmmu_defs.h
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -/*
> - * Software MMU support
> - *
> - * Declare helpers used by TCG for qemu_ld/st ops.
> - *
> - * Used by softmmu_exec.h, TCG targets and exec-all.h.
> - *
> - */
> -#ifndef SOFTMMU_DEFS_H
> -#define SOFTMMU_DEFS_H
> -
> -uint8_t helper_ret_ldb_mmu(CPUArchState *env, target_ulong addr,
> - int mmu_idx, uintptr_t retaddr);
> -uint16_t helper_ret_ldw_mmu(CPUArchState *env, target_ulong addr,
> - int mmu_idx, uintptr_t retaddr);
> -uint32_t helper_ret_ldl_mmu(CPUArchState *env, target_ulong addr,
> - int mmu_idx, uintptr_t retaddr);
> -uint64_t helper_ret_ldq_mmu(CPUArchState *env, target_ulong addr,
> - int mmu_idx, uintptr_t retaddr);
> -
> -void helper_ret_stb_mmu(CPUArchState *env, target_ulong addr, uint8_t val,
> - int mmu_idx, uintptr_t retaddr);
> -void helper_ret_stw_mmu(CPUArchState *env, target_ulong addr, uint16_t val,
> - int mmu_idx, uintptr_t retaddr);
> -void helper_ret_stl_mmu(CPUArchState *env, target_ulong addr, uint32_t val,
> - int mmu_idx, uintptr_t retaddr);
> -void helper_ret_stq_mmu(CPUArchState *env, target_ulong addr, uint64_t val,
> - int mmu_idx, uintptr_t retaddr);
> -
> -uint8_t helper_ldb_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> -uint16_t helper_ldw_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> -uint32_t helper_ldl_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> -uint64_t helper_ldq_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> -
> -void helper_stb_mmu(CPUArchState *env, target_ulong addr,
> - uint8_t val, int mmu_idx);
> -void helper_stw_mmu(CPUArchState *env, target_ulong addr,
> - uint16_t val, int mmu_idx);
> -void helper_stl_mmu(CPUArchState *env, target_ulong addr,
> - uint32_t val, int mmu_idx);
> -void helper_stq_mmu(CPUArchState *env, target_ulong addr,
> - uint64_t val, int mmu_idx);
> -
> -uint8_t helper_ldb_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> -uint16_t helper_ldw_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> -uint32_t helper_ldl_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> -uint64_t helper_ldq_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> -
> -#endif /* SOFTMMU_DEFS_H */
> diff --git a/include/exec/softmmu_exec.h b/include/exec/softmmu_exec.h
> index 3e4e886..6fde154 100644
> --- a/include/exec/softmmu_exec.h
> +++ b/include/exec/softmmu_exec.h
> @@ -19,7 +19,8 @@
> #define ldul_executive ldl_executive
> #define ldul_supervisor ldl_supervisor
>
> -#include "exec/softmmu_defs.h"
> +/* The memory helpers for tcg-generated code need tcg_target_long etc. */
> +#include "tcg.h"
>
> #define ACCESS_TYPE 0
> #define MEMSUFFIX MMU_MODE0_SUFFIX
> diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
> index 41a17f8..55ff700 100644
> --- a/tcg/aarch64/tcg-target.c
> +++ b/tcg/aarch64/tcg-target.c
> @@ -778,8 +778,6 @@ static inline void tcg_out_nop(TCGContext *s)
> }
>
> #ifdef CONFIG_SOFTMMU
> -#include "exec/softmmu_defs.h"
> -
> /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
> int mmu_idx) */
> static const void * const qemu_ld_helpers[4] = {
> diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
> index 6c4854d..6d084b3 100644
> --- a/tcg/arm/tcg-target.c
> +++ b/tcg/arm/tcg-target.c
> @@ -1058,8 +1058,6 @@ static inline void tcg_out_goto_label(TCGContext *s, int cond, int label_index)
>
> #ifdef CONFIG_SOFTMMU
>
> -#include "exec/softmmu_defs.h"
> -
> /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
> int mmu_idx) */
> static const void * const qemu_ld_helpers[4] = {
> diff --git a/tcg/hppa/tcg-target.c b/tcg/hppa/tcg-target.c
> index 68f77ba..bac1f12 100644
> --- a/tcg/hppa/tcg-target.c
> +++ b/tcg/hppa/tcg-target.c
> @@ -906,8 +906,6 @@ static void tcg_out_movcond(TCGContext *s, int cond, TCGArg ret,
> }
>
> #if defined(CONFIG_SOFTMMU)
> -#include "exec/softmmu_defs.h"
> -
> /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
> int mmu_idx) */
> static const void * const qemu_ld_helpers[4] = {
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index 84f17fe..4c98cc4 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -1022,9 +1022,6 @@ static void tcg_out_jmp(TCGContext *s, tcg_target_long dest)
> }
>
> #if defined(CONFIG_SOFTMMU)
> -
> -#include "exec/softmmu_defs.h"
> -
> /* helper signature: helper_ret_ld_mmu(CPUState *env, target_ulong addr,
> * int mmu_idx, uintptr_t ra)
> */
> diff --git a/tcg/ia64/tcg-target.c b/tcg/ia64/tcg-target.c
> index 2373d9e..56a1b3a 100644
> --- a/tcg/ia64/tcg-target.c
> +++ b/tcg/ia64/tcg-target.c
> @@ -1490,9 +1490,6 @@ static inline void tcg_out_movcond(TCGContext *s, TCGCond cond, TCGArg ret,
> }
>
> #if defined(CONFIG_SOFTMMU)
> -
> -#include "exec/softmmu_defs.h"
> -
> /* Load and compare a TLB entry, and return the result in (p6, p7).
> R2 is loaded with the address of the addend TLB entry.
> R57 is loaded with the address, zero extented on 32-bit targets. */
> diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
> index 793532e..a750d42 100644
> --- a/tcg/mips/tcg-target.c
> +++ b/tcg/mips/tcg-target.c
> @@ -919,9 +919,6 @@ static void tcg_out_setcond2(TCGContext *s, TCGCond cond, TCGReg ret,
> }
>
> #if defined(CONFIG_SOFTMMU)
> -
> -#include "exec/softmmu_defs.h"
> -
> /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
> int mmu_idx) */
> static const void * const qemu_ld_helpers[4] = {
> diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
> index 453ab6b..9a73d06 100644
> --- a/tcg/ppc/tcg-target.c
> +++ b/tcg/ppc/tcg-target.c
> @@ -549,8 +549,6 @@ static void add_qemu_ldst_label (TCGContext *s,
> label->label_ptr[0] = label_ptr;
> }
>
> -#include "exec/softmmu_defs.h"
> -
> /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
> int mmu_idx) */
> static const void * const qemu_ld_helpers[4] = {
> diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
> index 0678de2..ec067e6 100644
> --- a/tcg/ppc64/tcg-target.c
> +++ b/tcg/ppc64/tcg-target.c
> @@ -750,9 +750,6 @@ static void tcg_out_ldsta(TCGContext *s, TCGReg ret, TCGReg addr,
> }
>
> #if defined (CONFIG_SOFTMMU)
> -
> -#include "exec/softmmu_defs.h"
> -
> /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
> int mmu_idx) */
> static const void * const qemu_ld_helpers[4] = {
> diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.c
> index f229f1c..5b556fe 100644
> --- a/tcg/s390/tcg-target.c
> +++ b/tcg/s390/tcg-target.c
> @@ -315,9 +315,6 @@ static const uint8_t tcg_cond_to_ltr_cond[] = {
> };
>
> #ifdef CONFIG_SOFTMMU
> -
> -#include "exec/softmmu_defs.h"
> -
> /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
> int mmu_idx) */
> static const void * const qemu_ld_helpers[4] = {
> diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
> index 5bfd29c..b1e3fc1 100644
> --- a/tcg/sparc/tcg-target.c
> +++ b/tcg/sparc/tcg-target.c
> @@ -831,8 +831,6 @@ static void tcg_target_qemu_prologue(TCGContext *s)
>
> #if defined(CONFIG_SOFTMMU)
>
> -#include "exec/softmmu_defs.h"
> -
> /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
> int mmu_idx) */
> static const void * const qemu_ld_helpers[4] = {
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index f3f9889..eeff6b7 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -21,6 +21,10 @@
> * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> * THE SOFTWARE.
> */
> +
> +#ifndef TCG_H
> +#define TCG_H
> +
> #include "qemu-common.h"
>
> /* Target word size (must be identical to pointer size). */
> @@ -741,3 +745,42 @@ void tcg_register_jit(void *buf, size_t buf_size);
> /* Generate TB finalization at the end of block */
> void tcg_out_tb_finalize(TCGContext *s);
> #endif
> +
> +/*
> + * Memory helpers that will be used by TCG generated code.
> + */
> +#ifdef CONFIG_SOFTMMU
> +uint8_t helper_ret_ldb_mmu(CPUArchState *env, target_ulong addr,
> + int mmu_idx, uintptr_t retaddr);
> +uint16_t helper_ret_ldw_mmu(CPUArchState *env, target_ulong addr,
> + int mmu_idx, uintptr_t retaddr);
> +uint32_t helper_ret_ldl_mmu(CPUArchState *env, target_ulong addr,
> + int mmu_idx, uintptr_t retaddr);
> +uint64_t helper_ret_ldq_mmu(CPUArchState *env, target_ulong addr,
> + int mmu_idx, uintptr_t retaddr);
> +
> +void helper_ret_stb_mmu(CPUArchState *env, target_ulong addr, uint8_t val,
> + int mmu_idx, uintptr_t retaddr);
> +void helper_ret_stw_mmu(CPUArchState *env, target_ulong addr, uint16_t val,
> + int mmu_idx, uintptr_t retaddr);
> +void helper_ret_stl_mmu(CPUArchState *env, target_ulong addr, uint32_t val,
> + int mmu_idx, uintptr_t retaddr);
> +void helper_ret_stq_mmu(CPUArchState *env, target_ulong addr, uint64_t val,
> + int mmu_idx, uintptr_t retaddr);
> +
> +uint8_t helper_ldb_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> +uint16_t helper_ldw_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> +uint32_t helper_ldl_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> +uint64_t helper_ldq_mmu(CPUArchState *env, target_ulong addr, int mmu_idx);
> +
> +void helper_stb_mmu(CPUArchState *env, target_ulong addr,
> + uint8_t val, int mmu_idx);
> +void helper_stw_mmu(CPUArchState *env, target_ulong addr,
> + uint16_t val, int mmu_idx);
> +void helper_stl_mmu(CPUArchState *env, target_ulong addr,
> + uint32_t val, int mmu_idx);
> +void helper_stq_mmu(CPUArchState *env, target_ulong addr,
> + uint64_t val, int mmu_idx);
> +#endif /* CONFIG_SOFTMMU */
> +
> +#endif /* TCG_H */
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 6/7] tcg: Introduce zero and sign-extended versions of load helpers
2013-08-29 22:05 [Qemu-devel] [PATCH v2 0/7] Further tcg ldst improvements Richard Henderson
` (4 preceding siblings ...)
2013-08-29 22:05 ` [Qemu-devel] [PATCH v2 5/7] exec: Split softmmu_defs.h Richard Henderson
@ 2013-08-29 22:06 ` Richard Henderson
2013-08-30 16:55 ` Aurelien Jarno
2013-08-29 22:06 ` [Qemu-devel] [PATCH v2 7/7] tcg-i386: Make use of zero-extended memory helper routines Richard Henderson
6 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2013-08-29 22:06 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
include/exec/softmmu_template.h | 58 ++++++++++++++++++++++++++++++++---------
tcg/i386/tcg-target.c | 6 ++---
tcg/tcg.h | 21 ++++++++++-----
3 files changed, 64 insertions(+), 21 deletions(-)
diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
index f9922e2..5bbc56a 100644
--- a/include/exec/softmmu_template.h
+++ b/include/exec/softmmu_template.h
@@ -29,23 +29,39 @@
#if DATA_SIZE == 8
#define SUFFIX q
#define LSUFFIX q
-#define DATA_TYPE uint64_t
+#define SDATA_TYPE int64_t
#elif DATA_SIZE == 4
#define SUFFIX l
#define LSUFFIX l
-#define DATA_TYPE uint32_t
+#define SDATA_TYPE int32_t
#elif DATA_SIZE == 2
#define SUFFIX w
#define LSUFFIX uw
-#define DATA_TYPE uint16_t
+#define SDATA_TYPE int16_t
#elif DATA_SIZE == 1
#define SUFFIX b
#define LSUFFIX ub
-#define DATA_TYPE uint8_t
+#define SDATA_TYPE int8_t
#else
#error unsupported data size
#endif
+#define DATA_TYPE glue(u, SDATA_TYPE)
+
+/* For the benefit of TCG generated code, we want to avoid the complication
+ of ABI-specific return type promotion and always return a value extended
+ to the register size of the host. This is tcg_target_long, except in the
+ case of a 32-bit host and 64-bit data, and for that we always have
+ uint64_t. Don't bother with this widened value for SOFTMMU_CODE_ACCESS. */
+#if defined(SOFTMMU_CODE_ACCESS) || DATA_SIZE == 8
+# define WORD_TYPE DATA_TYPE
+# define USUFFIX SUFFIX
+#else
+# define WORD_TYPE tcg_target_ulong
+# define USUFFIX glue(u, SUFFIX)
+# define SSUFFIX glue(s, SUFFIX)
+#endif
+
#ifdef SOFTMMU_CODE_ACCESS
#define READ_ACCESS_TYPE 2
#define ADDR_READ addr_code
@@ -77,10 +93,10 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
#ifdef SOFTMMU_CODE_ACCESS
static
#endif
-DATA_TYPE
-glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env,
- target_ulong addr, int mmu_idx,
- uintptr_t retaddr)
+WORD_TYPE
+glue(glue(helper_ret_ld, USUFFIX), MMUSUFFIX)(CPUArchState *env,
+ target_ulong addr, int mmu_idx,
+ uintptr_t retaddr)
{
int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
target_ulong tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ;
@@ -126,9 +142,9 @@ glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env,
addr2 = addr1 + DATA_SIZE;
/* Note the adjustment at the beginning of the function.
Undo that for the recursion. */
- res1 = glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)
+ res1 = glue(glue(helper_ret_ld, USUFFIX), MMUSUFFIX)
(env, addr1, mmu_idx, retaddr + GETPC_ADJ);
- res2 = glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)
+ res2 = glue(glue(helper_ret_ld, USUFFIX), MMUSUFFIX)
(env, addr2, mmu_idx, retaddr + GETPC_ADJ);
shift = (addr & (DATA_SIZE - 1)) * 8;
#ifdef TARGET_WORDS_BIGENDIAN
@@ -147,19 +163,33 @@ glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env,
#endif
haddr = addr + env->tlb_table[mmu_idx][index].addend;
- return glue(glue(ld, LSUFFIX), _raw)((uint8_t *)haddr);
+ /* Note that ldl_raw is defined with type "int". */
+ return (DATA_TYPE) glue(glue(ld, LSUFFIX), _raw)((uint8_t *)haddr);
}
DATA_TYPE
glue(glue(helper_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
int mmu_idx)
{
- return glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(env, addr, mmu_idx,
+ return glue(glue(helper_ret_ld, USUFFIX), MMUSUFFIX)(env, addr, mmu_idx,
GETRA_EXT());
}
#ifndef SOFTMMU_CODE_ACCESS
+/* Provide signed versions of the load routines as well. We can of course
+ avoid this for 64-bit data, or for 32-bit data on 32-bit host. */
+#if DATA_SIZE * 8 < TCG_TARGET_REG_BITS
+WORD_TYPE
+glue(glue(helper_ret_ld, SSUFFIX), MMUSUFFIX)(CPUArchState *env,
+ target_ulong addr, int mmu_idx,
+ uintptr_t retaddr)
+{
+ return (SDATA_TYPE) glue(glue(helper_ret_ld, USUFFIX), MMUSUFFIX)
+ (env, addr, mmu_idx, retaddr);
+}
+#endif
+
static inline void glue(io_write, SUFFIX)(CPUArchState *env,
hwaddr physaddr,
DATA_TYPE val,
@@ -267,3 +297,7 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
#undef LSUFFIX
#undef DATA_SIZE
#undef ADDR_READ
+#undef WORD_TYPE
+#undef SDATA_TYPE
+#undef USUFFIX
+#undef SSUFFIX
diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index 4c98cc4..5aee0fa 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -1026,9 +1026,9 @@ static void tcg_out_jmp(TCGContext *s, tcg_target_long dest)
* int mmu_idx, uintptr_t ra)
*/
static const void * const qemu_ld_helpers[4] = {
- helper_ret_ldb_mmu,
- helper_ret_ldw_mmu,
- helper_ret_ldl_mmu,
+ helper_ret_ldub_mmu,
+ helper_ret_lduw_mmu,
+ helper_ret_ldul_mmu,
helper_ret_ldq_mmu,
};
diff --git a/tcg/tcg.h b/tcg/tcg.h
index eeff6b7..27f6ee5 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -750,15 +750,24 @@ void tcg_out_tb_finalize(TCGContext *s);
* Memory helpers that will be used by TCG generated code.
*/
#ifdef CONFIG_SOFTMMU
-uint8_t helper_ret_ldb_mmu(CPUArchState *env, target_ulong addr,
- int mmu_idx, uintptr_t retaddr);
-uint16_t helper_ret_ldw_mmu(CPUArchState *env, target_ulong addr,
- int mmu_idx, uintptr_t retaddr);
-uint32_t helper_ret_ldl_mmu(CPUArchState *env, target_ulong addr,
- int mmu_idx, uintptr_t retaddr);
+/* Value zero-extended to tcg register size. */
+tcg_target_ulong helper_ret_ldub_mmu(CPUArchState *env, target_ulong addr,
+ int mmu_idx, uintptr_t retaddr);
+tcg_target_ulong helper_ret_lduw_mmu(CPUArchState *env, target_ulong addr,
+ int mmu_idx, uintptr_t retaddr);
+tcg_target_ulong helper_ret_ldul_mmu(CPUArchState *env, target_ulong addr,
+ int mmu_idx, uintptr_t retaddr);
uint64_t helper_ret_ldq_mmu(CPUArchState *env, target_ulong addr,
int mmu_idx, uintptr_t retaddr);
+/* Value sign-extended to tcg register size. */
+tcg_target_ulong helper_ret_ldsb_mmu(CPUArchState *env, target_ulong addr,
+ int mmu_idx, uintptr_t retaddr);
+tcg_target_ulong helper_ret_ldsw_mmu(CPUArchState *env, target_ulong addr,
+ int mmu_idx, uintptr_t retaddr);
+tcg_target_ulong helper_ret_ldsl_mmu(CPUArchState *env, target_ulong addr,
+ int mmu_idx, uintptr_t retaddr);
+
void helper_ret_stb_mmu(CPUArchState *env, target_ulong addr, uint8_t val,
int mmu_idx, uintptr_t retaddr);
void helper_ret_stw_mmu(CPUArchState *env, target_ulong addr, uint16_t val,
--
1.8.1.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 6/7] tcg: Introduce zero and sign-extended versions of load helpers
2013-08-29 22:06 ` [Qemu-devel] [PATCH v2 6/7] tcg: Introduce zero and sign-extended versions of load helpers Richard Henderson
@ 2013-08-30 16:55 ` Aurelien Jarno
2013-08-30 17:20 ` Richard Henderson
0 siblings, 1 reply; 20+ messages in thread
From: Aurelien Jarno @ 2013-08-30 16:55 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Thu, Aug 29, 2013 at 03:06:00PM -0700, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> include/exec/softmmu_template.h | 58 ++++++++++++++++++++++++++++++++---------
> tcg/i386/tcg-target.c | 6 ++---
> tcg/tcg.h | 21 ++++++++++-----
> 3 files changed, 64 insertions(+), 21 deletions(-)
>
> diff --git a/include/exec/softmmu_template.h b/include/exec/softmmu_template.h
> index f9922e2..5bbc56a 100644
> --- a/include/exec/softmmu_template.h
> +++ b/include/exec/softmmu_template.h
> @@ -29,23 +29,39 @@
> #if DATA_SIZE == 8
> #define SUFFIX q
> #define LSUFFIX q
> -#define DATA_TYPE uint64_t
> +#define SDATA_TYPE int64_t
> #elif DATA_SIZE == 4
> #define SUFFIX l
> #define LSUFFIX l
> -#define DATA_TYPE uint32_t
> +#define SDATA_TYPE int32_t
> #elif DATA_SIZE == 2
> #define SUFFIX w
> #define LSUFFIX uw
> -#define DATA_TYPE uint16_t
> +#define SDATA_TYPE int16_t
> #elif DATA_SIZE == 1
> #define SUFFIX b
> #define LSUFFIX ub
> -#define DATA_TYPE uint8_t
> +#define SDATA_TYPE int8_t
> #else
> #error unsupported data size
> #endif
>
> +#define DATA_TYPE glue(u, SDATA_TYPE)
> +
> +/* For the benefit of TCG generated code, we want to avoid the complication
> + of ABI-specific return type promotion and always return a value extended
> + to the register size of the host. This is tcg_target_long, except in the
> + case of a 32-bit host and 64-bit data, and for that we always have
> + uint64_t. Don't bother with this widened value for SOFTMMU_CODE_ACCESS. */
> +#if defined(SOFTMMU_CODE_ACCESS) || DATA_SIZE == 8
> +# define WORD_TYPE DATA_TYPE
> +# define USUFFIX SUFFIX
> +#else
> +# define WORD_TYPE tcg_target_ulong
> +# define USUFFIX glue(u, SUFFIX)
> +# define SSUFFIX glue(s, SUFFIX)
> +#endif
> +
> #ifdef SOFTMMU_CODE_ACCESS
> #define READ_ACCESS_TYPE 2
> #define ADDR_READ addr_code
> @@ -77,10 +93,10 @@ static inline DATA_TYPE glue(io_read, SUFFIX)(CPUArchState *env,
> #ifdef SOFTMMU_CODE_ACCESS
> static
> #endif
> -DATA_TYPE
> -glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env,
> - target_ulong addr, int mmu_idx,
> - uintptr_t retaddr)
> +WORD_TYPE
> +glue(glue(helper_ret_ld, USUFFIX), MMUSUFFIX)(CPUArchState *env,
> + target_ulong addr, int mmu_idx,
> + uintptr_t retaddr)
> {
> int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
> target_ulong tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ;
> @@ -126,9 +142,9 @@ glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env,
> addr2 = addr1 + DATA_SIZE;
> /* Note the adjustment at the beginning of the function.
> Undo that for the recursion. */
> - res1 = glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)
> + res1 = glue(glue(helper_ret_ld, USUFFIX), MMUSUFFIX)
> (env, addr1, mmu_idx, retaddr + GETPC_ADJ);
> - res2 = glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)
> + res2 = glue(glue(helper_ret_ld, USUFFIX), MMUSUFFIX)
> (env, addr2, mmu_idx, retaddr + GETPC_ADJ);
> shift = (addr & (DATA_SIZE - 1)) * 8;
> #ifdef TARGET_WORDS_BIGENDIAN
> @@ -147,19 +163,33 @@ glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env,
> #endif
>
> haddr = addr + env->tlb_table[mmu_idx][index].addend;
> - return glue(glue(ld, LSUFFIX), _raw)((uint8_t *)haddr);
> + /* Note that ldl_raw is defined with type "int". */
> + return (DATA_TYPE) glue(glue(ld, LSUFFIX), _raw)((uint8_t *)haddr);
> }
>
> DATA_TYPE
> glue(glue(helper_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
> int mmu_idx)
> {
> - return glue(glue(helper_ret_ld, SUFFIX), MMUSUFFIX)(env, addr, mmu_idx,
> + return glue(glue(helper_ret_ld, USUFFIX), MMUSUFFIX)(env, addr, mmu_idx,
> GETRA_EXT());
> }
>
> #ifndef SOFTMMU_CODE_ACCESS
>
> +/* Provide signed versions of the load routines as well. We can of course
> + avoid this for 64-bit data, or for 32-bit data on 32-bit host. */
> +#if DATA_SIZE * 8 < TCG_TARGET_REG_BITS
> +WORD_TYPE
> +glue(glue(helper_ret_ld, SSUFFIX), MMUSUFFIX)(CPUArchState *env,
> + target_ulong addr, int mmu_idx,
> + uintptr_t retaddr)
> +{
> + return (SDATA_TYPE) glue(glue(helper_ret_ld, USUFFIX), MMUSUFFIX)
> + (env, addr, mmu_idx, retaddr);
> +}
> +#endif
> +
> static inline void glue(io_write, SUFFIX)(CPUArchState *env,
> hwaddr physaddr,
> DATA_TYPE val,
> @@ -267,3 +297,7 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
> #undef LSUFFIX
> #undef DATA_SIZE
> #undef ADDR_READ
> +#undef WORD_TYPE
> +#undef SDATA_TYPE
> +#undef USUFFIX
> +#undef SSUFFIX
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index 4c98cc4..5aee0fa 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -1026,9 +1026,9 @@ static void tcg_out_jmp(TCGContext *s, tcg_target_long dest)
> * int mmu_idx, uintptr_t ra)
> */
> static const void * const qemu_ld_helpers[4] = {
> - helper_ret_ldb_mmu,
> - helper_ret_ldw_mmu,
> - helper_ret_ldl_mmu,
> + helper_ret_ldub_mmu,
> + helper_ret_lduw_mmu,
> + helper_ret_ldul_mmu,
> helper_ret_ldq_mmu,
> };
>
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index eeff6b7..27f6ee5 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -750,15 +750,24 @@ void tcg_out_tb_finalize(TCGContext *s);
> * Memory helpers that will be used by TCG generated code.
> */
> #ifdef CONFIG_SOFTMMU
> -uint8_t helper_ret_ldb_mmu(CPUArchState *env, target_ulong addr,
> - int mmu_idx, uintptr_t retaddr);
> -uint16_t helper_ret_ldw_mmu(CPUArchState *env, target_ulong addr,
> - int mmu_idx, uintptr_t retaddr);
> -uint32_t helper_ret_ldl_mmu(CPUArchState *env, target_ulong addr,
> - int mmu_idx, uintptr_t retaddr);
> +/* Value zero-extended to tcg register size. */
> +tcg_target_ulong helper_ret_ldub_mmu(CPUArchState *env, target_ulong addr,
> + int mmu_idx, uintptr_t retaddr);
> +tcg_target_ulong helper_ret_lduw_mmu(CPUArchState *env, target_ulong addr,
> + int mmu_idx, uintptr_t retaddr);
> +tcg_target_ulong helper_ret_ldul_mmu(CPUArchState *env, target_ulong addr,
> + int mmu_idx, uintptr_t retaddr);
> uint64_t helper_ret_ldq_mmu(CPUArchState *env, target_ulong addr,
> int mmu_idx, uintptr_t retaddr);
>
> +/* Value sign-extended to tcg register size. */
> +tcg_target_ulong helper_ret_ldsb_mmu(CPUArchState *env, target_ulong addr,
> + int mmu_idx, uintptr_t retaddr);
> +tcg_target_ulong helper_ret_ldsw_mmu(CPUArchState *env, target_ulong addr,
> + int mmu_idx, uintptr_t retaddr);
> +tcg_target_ulong helper_ret_ldsl_mmu(CPUArchState *env, target_ulong addr,
> + int mmu_idx, uintptr_t retaddr);
> +
> void helper_ret_stb_mmu(CPUArchState *env, target_ulong addr, uint8_t val,
> int mmu_idx, uintptr_t retaddr);
> void helper_ret_stw_mmu(CPUArchState *env, target_ulong addr, uint16_t val,
While it works for x86 and some other architectures, it makes the
assumption that only part of the register can be used later by the TCG
code. It won't be the case if we later (and I hope we will) implement a
MIPS64 TCG target. In that case, a 32-bit value has to be returned
signed extended, which won't be the case for example for a 32-bit guest
loading a 16-bit unsigned value.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 6/7] tcg: Introduce zero and sign-extended versions of load helpers
2013-08-30 16:55 ` Aurelien Jarno
@ 2013-08-30 17:20 ` Richard Henderson
2013-08-30 19:12 ` Aurelien Jarno
0 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2013-08-30 17:20 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel
On 08/30/2013 09:55 AM, Aurelien Jarno wrote:
> While it works for x86 and some other architectures, it makes the
> assumption that only part of the register can be used later by the TCG
> code. It won't be the case if we later (and I hope we will) implement a
> MIPS64 TCG target. In that case, a 32-bit value has to be returned
> signed extended, which won't be the case for example for a 32-bit guest
> loading a 16-bit unsigned value.
This doesn't break the mips64 abi, since we'll be returning a 64-bit value, not
a 32-bit value that needs sign-extension.
Given a mips64 host with 32-bit guest, the sign-extension of the 32-bit load
can either happen by using helper_ret_ldsl_mmu in the table of helper
functions, or by using an sll insn instead of a move to put the value into
place at the end of the slow path.
I have more or less the same constraint in my as-yet unsubmitted Alpha backend.
r~
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/7] tcg: Introduce zero and sign-extended versions of load helpers
2013-08-30 17:20 ` Richard Henderson
@ 2013-08-30 19:12 ` Aurelien Jarno
2013-08-30 20:53 ` Richard Henderson
0 siblings, 1 reply; 20+ messages in thread
From: Aurelien Jarno @ 2013-08-30 19:12 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Fri, Aug 30, 2013 at 10:20:23AM -0700, Richard Henderson wrote:
> On 08/30/2013 09:55 AM, Aurelien Jarno wrote:
> > While it works for x86 and some other architectures, it makes the
> > assumption that only part of the register can be used later by the TCG
> > code. It won't be the case if we later (and I hope we will) implement a
> > MIPS64 TCG target. In that case, a 32-bit value has to be returned
> > signed extended, which won't be the case for example for a 32-bit guest
> > loading a 16-bit unsigned value.
>
> This doesn't break the mips64 abi, since we'll be returning a 64-bit value, not
> a 32-bit value that needs sign-extension.
>
> Given a mips64 host with 32-bit guest, the sign-extension of the 32-bit load
> can either happen by using helper_ret_ldsl_mmu in the table of helper
> functions, or by using an sll insn instead of a move to put the value into
> place at the end of the slow path.
That's indeed a possibility. That said while the MIPS64 ABI is then
still followed, it would have break a MIPS backend as the ABI between
the helper and the TCG code is broken.
I am therefore concerned that we might break some of our 64-bit
backends. x86-64 and ia64 should be fine, I don't know about aarch64,
ppc64, sparc64 or s390x.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/7] tcg: Introduce zero and sign-extended versions of load helpers
2013-08-30 19:12 ` Aurelien Jarno
@ 2013-08-30 20:53 ` Richard Henderson
2013-08-30 21:23 ` Aurelien Jarno
0 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2013-08-30 20:53 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel
On 08/30/2013 12:12 PM, Aurelien Jarno wrote:
> On Fri, Aug 30, 2013 at 10:20:23AM -0700, Richard Henderson wrote:
>> On 08/30/2013 09:55 AM, Aurelien Jarno wrote:
>>> While it works for x86 and some other architectures, it makes the
>>> assumption that only part of the register can be used later by the TCG
>>> code. It won't be the case if we later (and I hope we will) implement a
>>> MIPS64 TCG target. In that case, a 32-bit value has to be returned
>>> signed extended, which won't be the case for example for a 32-bit guest
>>> loading a 16-bit unsigned value.
>>
>> This doesn't break the mips64 abi, since we'll be returning a 64-bit value, not
>> a 32-bit value that needs sign-extension.
>>
>> Given a mips64 host with 32-bit guest, the sign-extension of the 32-bit load
>> can either happen by using helper_ret_ldsl_mmu in the table of helper
>> functions, or by using an sll insn instead of a move to put the value into
>> place at the end of the slow path.
>
> That's indeed a possibility. That said while the MIPS64 ABI is then
> still followed, it would have break a MIPS backend as the ABI between
> the helper and the TCG code is broken.
How's that? We're passing a value extended to tcg_target_ulong.
For the 32-bit mips backend running o32 (or even a theoretical n32 backend),
that type is uint32_t. That gets returned from C exactly how C returns that
type. For o32 it's the full width of the register, full stop. For n32, it
would be returned sign-extended in the 64-bit register.
Please explain exactly the failure mode you imagine, because I don't think
there is one.
> I am therefore concerned that we might break some of our 64-bit
> backends. x86-64 and ia64 should be fine, I don't know about aarch64,
> ppc64, sparc64 or s390x.
Nope, all 4 of those will be fine. Not least of which because the later 3
are still using the original helper functions, not the new helper_ret_* ones.
But certainly all 4 of those are, in the gcc sense, TRULY_NOOP_TRUNCATION
machines, meaning we can truncate to 32 bits merely by ignoring the garbage
in the high bits. In practice it means that they have different 32-bit and
64-bit comparison instructions.
r~
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/7] tcg: Introduce zero and sign-extended versions of load helpers
2013-08-30 20:53 ` Richard Henderson
@ 2013-08-30 21:23 ` Aurelien Jarno
2013-08-31 0:05 ` Richard Henderson
0 siblings, 1 reply; 20+ messages in thread
From: Aurelien Jarno @ 2013-08-30 21:23 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Fri, Aug 30, 2013 at 01:53:37PM -0700, Richard Henderson wrote:
> On 08/30/2013 12:12 PM, Aurelien Jarno wrote:
> > On Fri, Aug 30, 2013 at 10:20:23AM -0700, Richard Henderson wrote:
> >> On 08/30/2013 09:55 AM, Aurelien Jarno wrote:
> >>> While it works for x86 and some other architectures, it makes the
> >>> assumption that only part of the register can be used later by the TCG
> >>> code. It won't be the case if we later (and I hope we will) implement a
> >>> MIPS64 TCG target. In that case, a 32-bit value has to be returned
> >>> signed extended, which won't be the case for example for a 32-bit guest
> >>> loading a 16-bit unsigned value.
> >>
> >> This doesn't break the mips64 abi, since we'll be returning a 64-bit value, not
> >> a 32-bit value that needs sign-extension.
> >>
> >> Given a mips64 host with 32-bit guest, the sign-extension of the 32-bit load
> >> can either happen by using helper_ret_ldsl_mmu in the table of helper
> >> functions, or by using an sll insn instead of a move to put the value into
> >> place at the end of the slow path.
> >
> > That's indeed a possibility. That said while the MIPS64 ABI is then
> > still followed, it would have break a MIPS backend as the ABI between
> > the helper and the TCG code is broken.
Sorry I meant MIPS64 backend.
> How's that? We're passing a value extended to tcg_target_ulong.
>
> For the 32-bit mips backend running o32 (or even a theoretical n32 backend),
> that type is uint32_t. That gets returned from C exactly how C returns that
> type. For o32 it's the full width of the register, full stop. For n32, it
> would be returned sign-extended in the 64-bit register.
>
> Please explain exactly the failure mode you imagine, because I don't think
> there is one.
For MIPS64, to load a 32-bit value in a 32-bit guest, the slow path
would have called helper_ret_ldl_mmu(), returning and uint32_t, but then
signed extended to 64-bit in the register as per MIPS64 ABI.
With the new code helper_ret_ldl_mmu() would return a tcg_target_ulong
value, so zero extended to 64-bit. If this register is later used in a
32-bit op, it is not anymore sign extended, and the result is then
unpredictable.
As you said earlier the solution would have been to replace
helper_ret_ldl_mmu() by helper_ret_ldsl_mmu(), but it should have been
done in the same patch to not break things. I just wanted to make
sure we don't have the problem in an another target
> > I am therefore concerned that we might break some of our 64-bit
> > backends. x86-64 and ia64 should be fine, I don't know about aarch64,
> > ppc64, sparc64 or s390x.
>
> Nope, all 4 of those will be fine. Not least of which because the later 3
> are still using the original helper functions, not the new helper_ret_* ones.
>
> But certainly all 4 of those are, in the gcc sense, TRULY_NOOP_TRUNCATION
> machines, meaning we can truncate to 32 bits merely by ignoring the garbage
> in the high bits. In practice it means that they have different 32-bit and
> 64-bit comparison instructions.
If they are all fine, you get:
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 6/7] tcg: Introduce zero and sign-extended versions of load helpers
2013-08-30 21:23 ` Aurelien Jarno
@ 2013-08-31 0:05 ` Richard Henderson
0 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2013-08-31 0:05 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: qemu-devel
On 08/30/2013 02:23 PM, Aurelien Jarno wrote:
> As you said earlier the solution would have been to replace
> helper_ret_ldl_mmu() by helper_ret_ldsl_mmu(), but it should have been
> done in the same patch to not break things. I just wanted to make
> sure we don't have the problem in an another target
Ah, but that's where we're in luck: helper_ret_ldl_mmu is brand new.
It's all of a week old, introduced with the first set of these patches.
There are no legacy users.
r~
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 7/7] tcg-i386: Make use of zero-extended memory helper routines
2013-08-29 22:05 [Qemu-devel] [PATCH v2 0/7] Further tcg ldst improvements Richard Henderson
` (5 preceding siblings ...)
2013-08-29 22:06 ` [Qemu-devel] [PATCH v2 6/7] tcg: Introduce zero and sign-extended versions of load helpers Richard Henderson
@ 2013-08-29 22:06 ` Richard Henderson
2013-08-30 21:23 ` Aurelien Jarno
6 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2013-08-29 22:06 UTC (permalink / raw)
To: qemu-devel; +Cc: aurelien
For 8 and 16-bit unsigned loads, rely on the zero-extension
from the helper and use a smaller 32-bit move insn.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
tcg/i386/tcg-target.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index 5aee0fa..e9d6c49 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -1520,20 +1520,17 @@ static void tcg_out_qemu_ld_slow_path(TCGContext *s, TCGLabelQemuLdst *l)
case 1 | 4:
tcg_out_ext16s(s, data_reg, TCG_REG_EAX, P_REXW);
break;
- case 0:
- tcg_out_ext8u(s, data_reg, TCG_REG_EAX);
- break;
- case 1:
- tcg_out_ext16u(s, data_reg, TCG_REG_EAX);
- break;
- case 2:
- tcg_out_mov(s, TCG_TYPE_I32, data_reg, TCG_REG_EAX);
- break;
#if TCG_TARGET_REG_BITS == 64
case 2 | 4:
tcg_out_ext32s(s, data_reg, TCG_REG_EAX);
break;
#endif
+ case 0:
+ case 1:
+ /* Note that the helpers have zero-extended to tcg_target_long. */
+ case 2:
+ tcg_out_mov(s, TCG_TYPE_I32, data_reg, TCG_REG_EAX);
+ break;
case 3:
if (TCG_TARGET_REG_BITS == 64) {
tcg_out_mov(s, TCG_TYPE_I64, data_reg, TCG_REG_RAX);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Qemu-devel] [PATCH v2 7/7] tcg-i386: Make use of zero-extended memory helper routines
2013-08-29 22:06 ` [Qemu-devel] [PATCH v2 7/7] tcg-i386: Make use of zero-extended memory helper routines Richard Henderson
@ 2013-08-30 21:23 ` Aurelien Jarno
0 siblings, 0 replies; 20+ messages in thread
From: Aurelien Jarno @ 2013-08-30 21:23 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel
On Thu, Aug 29, 2013 at 03:06:01PM -0700, Richard Henderson wrote:
> For 8 and 16-bit unsigned loads, rely on the zero-extension
> from the helper and use a smaller 32-bit move insn.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
> tcg/i386/tcg-target.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
> index 5aee0fa..e9d6c49 100644
> --- a/tcg/i386/tcg-target.c
> +++ b/tcg/i386/tcg-target.c
> @@ -1520,20 +1520,17 @@ static void tcg_out_qemu_ld_slow_path(TCGContext *s, TCGLabelQemuLdst *l)
> case 1 | 4:
> tcg_out_ext16s(s, data_reg, TCG_REG_EAX, P_REXW);
> break;
> - case 0:
> - tcg_out_ext8u(s, data_reg, TCG_REG_EAX);
> - break;
> - case 1:
> - tcg_out_ext16u(s, data_reg, TCG_REG_EAX);
> - break;
> - case 2:
> - tcg_out_mov(s, TCG_TYPE_I32, data_reg, TCG_REG_EAX);
> - break;
> #if TCG_TARGET_REG_BITS == 64
> case 2 | 4:
> tcg_out_ext32s(s, data_reg, TCG_REG_EAX);
> break;
> #endif
> + case 0:
> + case 1:
> + /* Note that the helpers have zero-extended to tcg_target_long. */
> + case 2:
> + tcg_out_mov(s, TCG_TYPE_I32, data_reg, TCG_REG_EAX);
> + break;
> case 3:
> if (TCG_TARGET_REG_BITS == 64) {
> tcg_out_mov(s, TCG_TYPE_I64, data_reg, TCG_REG_RAX);
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 20+ messages in thread