[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/8] target-arm: A64: add support for ld/st p
From: |
C Fontana |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/8] target-arm: A64: add support for ld/st pair |
Date: |
Thu, 12 Dec 2013 13:14:09 +0100 |
On 12 December 2013 12:45, Peter Maydell <address@hidden> wrote:
> On 12 December 2013 11:43, Alex Bennée <address@hidden> wrote:
>>
>> address@hidden writes:
>>
>>> Hi,
>>>
>>> I saw a missing return below:
>> <snip>
>>>> + default: /* Failed decoder tree? */
>>>> + unallocated_encoding(s);
>>>> + break;
>>>> + }
>>>
>>> This doesn't seem right (break instead of return):
>>>
>>> default:
>>> unallocated_encoding(s);
>>> return;
>>> }
>>
>> Good catch. I suspect it never hits though (or risu doesn't generate
>> enough unallocated versions).
>
> Oh, I saw that and then forgot to mention it.
> Either the comment or the code is wrong -- if
> the decoder tree prevents us getting to this point
> with that value of index then we should be asserting,
> not calling unallocated_encoding(). If the encoding
> is really supposed to be unallocated then it's not
> a failure of the decoder tree that we got here,
> it's just another case we need to check. Which is it?
>
> thanks
> -- PMM
I think that there is more than the missing return:
we need to handle the case 0 as well, as it's a perfectly valid form
of a load/store pair:
it's the Load/Store no-allocate pair (offset) (LDNP, STNP).
So in my view we need to add a case 0 where we handle the load/store
no-allocate pair,
and no default case, or a default case where we assert(0) for unreachable code,
since all possible index values (2 bits) should be handled by the
switch statement.
Ref: C3.3 Loads and stores Table C3-3
Claudio
[Qemu-devel] [PATCH v2 7/8] target-arm: A64: add support for 3 src data proc insns, Peter Maydell, 2013/12/11
[Qemu-devel] [PATCH v2 3/8] target-arm: A64: add support for ld/st with reg offset, Peter Maydell, 2013/12/11
[Qemu-devel] [PATCH v2 6/8] target-arm: A64: add support for move wide instructions, Peter Maydell, 2013/12/11
[Qemu-devel] [PATCH v2 5/8] target-arm: A64: add support for add, addi, sub, subi, Peter Maydell, 2013/12/11