[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 7/9] tcg-ppc64: Define TCG_TARGET_INSN_UNIT_S
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v2 7/9] tcg-ppc64: Define TCG_TARGET_INSN_UNIT_SIZE |
Date: |
Tue, 1 Apr 2014 22:12:13 +0100 |
On 1 April 2014 21:53, Richard Henderson <address@hidden> wrote:
> And use tcg pointer differencing functions as appropriate.
>
> Signed-off-by: Richard Henderson <address@hidden>
Few minor nits and queries below.
> ---
> tcg/ppc64/tcg-target.c | 163
> ++++++++++++++++++++++++-------------------------
> tcg/ppc64/tcg-target.h | 1 +
> 2 files changed, 80 insertions(+), 84 deletions(-)
>
> diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
> index 06e440f..a199a96 100644
> --- a/tcg/ppc64/tcg-target.c
> +++ b/tcg/ppc64/tcg-target.c
> @@ -31,7 +31,7 @@
> #define TCG_CT_CONST_ZERO 0x1000
> #define TCG_CT_CONST_MONE 0x2000
>
> -static uint8_t *tb_ret_addr;
> +static tcg_insn_unit *tb_ret_addr;
>
> #if TARGET_LONG_BITS == 32
> #define LD_ADDR LWZ
> @@ -168,61 +168,53 @@ static inline bool in_range_b(tcg_target_long target)
> return target == sextract64(target, 0, 26);
> }
>
> -static uint32_t reloc_pc24_val(void *pc, tcg_target_long target)
> +static uint32_t reloc_pc24_val(void *pc, tcg_insn_unit *target)
> {
> - tcg_target_long disp;
> -
> - disp = target - (tcg_target_long)pc;
> + ptrdiff_t disp = tcg_ptr_byte_diff(target, pc);
> assert(in_range_b(disp));
> -
> return disp & 0x3fffffc;
> }
>
> -static void reloc_pc24(void *pc, tcg_target_long target)
> +static void reloc_pc24(tcg_insn_unit *pc, tcg_insn_unit *target)
> {
> - *(uint32_t *)pc = (*(uint32_t *)pc & ~0x3fffffc)
> - | reloc_pc24_val(pc, target);
> + *pc = (*pc & ~0x3fffffc) | reloc_pc24_val(pc, target);
> }
>
> -static uint16_t reloc_pc14_val(void *pc, tcg_target_long target)
> +static uint16_t reloc_pc14_val(void *pc, tcg_insn_unit *target)
Should be tcg_insn_unit *pc like the others I guess?
> {
> - tcg_target_long disp;
> -
> - disp = target - (tcg_target_long)pc;
> - if (disp != (int16_t) disp) {
> - tcg_abort();
> - }
> -
> + ptrdiff_t disp = tcg_ptr_byte_diff(target, pc);
> + assert(disp == (int16_t) disp);
> return disp & 0xfffc;
> }
>
> -static void reloc_pc14(void *pc, tcg_target_long target)
> +static void reloc_pc14(tcg_insn_unit *pc, tcg_insn_unit *target)
> {
> - *(uint32_t *)pc = (*(uint32_t *)pc & ~0xfffc) | reloc_pc14_val(pc,
> target);
> + *pc = (*pc & ~0xfffc) | reloc_pc14_val(pc, target);
> }
>
> static inline void tcg_out_b_noaddr(TCGContext *s, int insn)
> {
> - unsigned retrans = *(uint32_t *)s->code_ptr & 0x3fffffc;
> + unsigned retrans = *s->code_ptr & 0x3fffffc;
> tcg_out32(s, insn | retrans);
> }
>
> static inline void tcg_out_bc_noaddr(TCGContext *s, int insn)
> {
> - unsigned retrans = *(uint32_t *)s->code_ptr & 0xfffc;
> + unsigned retrans = *s->code_ptr & 0xfffc;
> tcg_out32(s, insn | retrans);
> }
>
> -static void patch_reloc(uint8_t *code_ptr, int type,
> +static void patch_reloc(tcg_insn_unit *code_ptr, int type,
> intptr_t value, intptr_t addend)
> {
> - value += addend;
> + /* Note that we always use 0 for addend in calls to tcg_out_reloc. */
Is this comment saying "assert(addend == 0)" ?
> + tcg_insn_unit *target = (tcg_insn_unit *)value;
> switch (type) {
> case R_PPC_REL14:
> - reloc_pc14(code_ptr, value);
> + reloc_pc14(code_ptr, target);
> break;
> case R_PPC_REL24:
> - reloc_pc24(code_ptr, value);
> + reloc_pc24(code_ptr, target);
> break;
> default:
> tcg_abort();
> @@ -836,7 +832,7 @@ static const uint32_t qemu_exts_opc[4] = {
> /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
> * int mmu_idx, uintptr_t ra)
> */
> -static const void * const qemu_ld_helpers[16] = {
> +static void * const qemu_ld_helpers[16] = {
> [MO_UB] = helper_ret_ldub_mmu,
> [MO_LEUW] = helper_le_lduw_mmu,
> [MO_LEUL] = helper_le_ldul_mmu,
> @@ -849,7 +845,7 @@ static const void * const qemu_ld_helpers[16] = {
> /* helper signature: helper_st_mmu(CPUState *env, target_ulong addr,
> * uintxx_t val, int mmu_idx, uintptr_t ra)
> */
> -static const void * const qemu_st_helpers[16] = {
> +static void * const qemu_st_helpers[16] = {
Why do we lose the extra 'const' here?
> @@ -1115,7 +1111,8 @@ static void tcg_target_qemu_prologue(TCGContext *s)
> #ifndef __APPLE__
> /* First emit adhoc function descriptor */
> tcg_out64(s, (uint64_t)s->code_ptr + 24); /* entry point */
> - s->code_ptr += 16; /* skip TOC and environment pointer */
> + tcg_out64(s, 0); /* toc */
> + tcg_out64(s, 0); /* environment pointer */
This is a behaviour change, right? Is it a bugfix or just a "doesn't
matter if we write zeros here or not" thing?
thanks
-- PMM
- [Qemu-devel] [PATCH v2 0/9] tcg: tidy the type of code_ptr, Richard Henderson, 2014/04/01
- [Qemu-devel] [PATCH v2 1/9] exec-all.h: Use stl_p to avoid undefined behaviour patching x86 jumps, Richard Henderson, 2014/04/01
- [Qemu-devel] [PATCH v2 2/9] tcg: Avoid stores to unaligned addresses, Richard Henderson, 2014/04/01
- [Qemu-devel] [PATCH v2 3/9] tcg: Avoid undefined behaviour patching code at unaligned addresses, Richard Henderson, 2014/04/01
- [Qemu-devel] [PATCH v2 4/9] tcg: Introduce byte pointer arithmetic helpers, Richard Henderson, 2014/04/01
- [Qemu-devel] [PATCH v2 5/9] tcg: Define tcg_insn_unit for code pointers, Richard Henderson, 2014/04/01
- [Qemu-devel] [PATCH v2 6/9] tcg-i386: Define TCG_TARGET_INSN_UNIT_SIZE, Richard Henderson, 2014/04/01
- [Qemu-devel] [PATCH v2 7/9] tcg-ppc64: Define TCG_TARGET_INSN_UNIT_SIZE, Richard Henderson, 2014/04/01
- Re: [Qemu-devel] [PATCH v2 7/9] tcg-ppc64: Define TCG_TARGET_INSN_UNIT_SIZE,
Peter Maydell <=
- [Qemu-devel] [PATCH v2 8/9] tcg-ppc: Define TCG_TARGET_INSN_UNIT_SIZE, Richard Henderson, 2014/04/01
- [Qemu-devel] [PATCH v2 9/9] tcg-sparc: Define TCG_TARGET_INSN_UNIT_SIZE, Richard Henderson, 2014/04/01
- Re: [Qemu-devel] [PATCH v2 0/9] tcg: tidy the type of code_ptr, Peter Maydell, 2014/04/01