On 12 December 2013 12:45, Peter Maydell <peter.mayd...@linaro.org> wrote: > On 12 December 2013 11:43, Alex Bennée <alex.ben...@linaro.org> wrote: >> >> claudio.font...@linaro.org 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
