Re: [Qemu-devel] [PATCH 4/4] target-arm: neon fix

[Laurent Desnogues <laurent.desnogues@gmail.com>]

On Sun, Feb 7, 2010 at 1:54 PM, Laurent Desnogues
<laurent.desnogues@gmail.com> wrote:
> On Fri, Feb 5, 2010 at 4:52 PM, Riku Voipio <riku.voipio@iki.fi> wrote:
>> From: Juha Riihimäki <juha.riihimaki@nokia.com>
>>
>> add an extra check in "two registers and a shift" to ensure element
>> size decoding logic cannot fail.
>>
>> Signed-off-by: Juha Riihimäki <juha.riihimaki@nokia.com>
>> Signed-off-by: Riku Voipio <riku.voipio@nokia.com>
>> ---
>>  target-arm/translate.c |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/target-arm/translate.c b/target-arm/translate.c
>> index 743b846..8bba034 100644
>> --- a/target-arm/translate.c
>> +++ b/target-arm/translate.c
>> @@ -4567,8 +4567,9 @@ static int disas_neon_data_insn(CPUState * env, DisasContext *s, uint32_t insn)
>>                 size = 3;
>>             } else {
>>                 size = 2;
>> -                while (size && (insn & (1 << (size + 19))) == 0)
>> +                while (size && (insn & (1 << (size + 19))) == 0) {
>>                     size--;
>> +                }
>>             }
>>             shift = (insn >> 16) & ((1 << (3 + size)) - 1);
>>             /* To avoid excessive dumplication of ops we implement shift
>
> I think there's a patch ordering problem that makes
> the comment and the change not agree :-)

BTW I don't think adding the check for size is needed
here.  The encoding at that point looks like this:

 3322222222221111111111
 10987654321098765432109876543210
 1111001_1___1______________1____
 1111001_1__1_______________1____
 1111001_1_1________________1____

so it will stop for size == 0 given that bit 19 will have to
be set.


Laurent