On Sun, Feb 7, 2010 at 1:54 PM, Laurent Desnogues <laurent.desnog...@gmail.com> wrote: > On Fri, Feb 5, 2010 at 4:52 PM, Riku Voipio <riku.voi...@iki.fi> wrote: >> From: Juha Riihimäki <juha.riihim...@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.riihim...@nokia.com> >> Signed-off-by: Riku Voipio <riku.voi...@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