[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 07/16] tcg-ppc64: DefineTCG_TARGET_INSN_UNIT_
From: |
Richard Henderson |
Subject: |
Re: [Qemu-devel] [PATCH v3 07/16] tcg-ppc64: DefineTCG_TARGET_INSN_UNIT_SIZEE |
Date: |
Tue, 29 Apr 2014 09:54:53 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 |
On 04/29/2014 08:41 AM, Alex Bennée wrote:
>
> Richard Henderson <address@hidden> writes:
>
>> On 04/29/2014 04:25 AM, Alex Bennée wrote:
>>>> +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);
>>>> }
>>>
>>> Should 0x3fffffc be a #define'd mask? Can the PC ever actually be
>>> non-word aligned?
>>
>> No, it can't be unaligned. The 0x3fffffc is really the field into
>> which the offset is being placed. I don't think a define is helpful
>> really, since this is the only place it's used.
>
> Ahh ok that makes sense. I was thrown because the masking pattern seems
> to occur all around the ppc tcg code:
>
> 16:40 address@hidden/x86_64 [qemu.git] >git grep "0x3fffffc"
> disas/ppc.c: { 0x3fffffc, 0, NULL, NULL, PPC_OPERAND_RELATIVE |
> PPC_OPERAND_SIGNED },
> disas/ppc.c: { 0x3fffffc, 0, NULL, NULL, PPC_OPERAND_ABSOLUTE |
> PPC_OPERAND_SIGNED },
> tcg/ppc/tcg-target.c: return disp & 0x3fffffc;
> tcg/ppc/tcg-target.c: *(uint32_t *) pc = (*(uint32_t *) pc & ~0x3fffffc)
> tcg/ppc/tcg-target.c: tcg_out32 (s, B | (disp & 0x3fffffc) | mask);
> tcg/ppc/tcg-target.c: tcg_out32 (s, B | (val & 0x3fffffc));
> tcg/ppc64/tcg-target.c: return disp & 0x3fffffc;
> tcg/ppc64/tcg-target.c: *(uint32_t *)pc = (*(uint32_t *)pc & ~0x3fffffc)
> tcg/ppc64/tcg-target.c: unsigned retrans = *(uint32_t *)s->code_ptr &
> 0x3fffffc;
> tcg/ppc64/tcg-target.c: tcg_out32(s, B | (disp & 0x3fffffc) | mask);
Well, its true that it's going to be replicated between the disassembler and
the two ppc backends. I'm slightly surprised that it appears more than twice
for each ppc backend, but I suppose that just means there's more room to tidy
up.
I do think that's out of scope for this patch set though.
r~
- [Qemu-devel] [PATCH v3 03/16] tcg: Avoid undefined behaviour patching code at unaligned addresses, (continued)
[Qemu-devel] [PATCH v3 09/16] tcg-sparc: Define TCG_TARGET_INSN_UNIT_SIZE, Richard Henderson, 2014/04/28
[Qemu-devel] [PATCH v3 10/16] tcg-arm: Define TCG_TARGET_INSN_UNIT_SIZE, Richard Henderson, 2014/04/28
[Qemu-devel] [PATCH v3 11/16] tcg-aarch64: Define TCG_TARGET_INSN_UNIT_SIZE, Richard Henderson, 2014/04/28
[Qemu-devel] [PATCH v3 13/16] tcg-ia64: Define TCG_TARGET_INSN_UNIT_SIZE, Richard Henderson, 2014/04/28
[Qemu-devel] [PATCH v3 12/16] tcg-s390: Define TCG_TARGET_INSN_UNIT_SIZE, Richard Henderson, 2014/04/28
[Qemu-devel] [PATCH v3 14/16] tcg-mips: Define TCG_TARGET_INSN_UNIT_SIZE, Richard Henderson, 2014/04/28
[Qemu-devel] [PATCH v3 15/16] tci: Define TCG_TARGET_INSN_UNIT_SIZE, Richard Henderson, 2014/04/28
[Qemu-devel] [PATCH v3 16/16] tcg: Require TCG_TARGET_INSN_UNIT_SIZE, Richard Henderson, 2014/04/28