target-arm: decode TBB/TBH more thoroughly

This avoids other opcodes being incorrectly decoded as TBB/TBH.
The LDA/STL instructions new in ARMv8 use this space.

Signed-off-by: Mans Rullgard <mans@mansr.com>
---
This was previously sent as part of the LDA/STL patch.  Separating it
seems clearer.
---
 target-arm/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On 17 June 2013 14:00, Mans Rullgard <mans@mansr.com> wrote:
> This avoids other opcodes being incorrectly decoded as TBB/TBH.
> The LDA/STL instructions new in ARMv8 use this space.
>
> Signed-off-by: Mans Rullgard <mans@mansr.com>
> ---
> This was previously sent as part of the LDA/STL patch.  Separating it
> seems clearer.
> ---
>  target-arm/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 3ffe7b8..bc41f7e 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -8126,7 +8126,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
>                      gen_store_exclusive(s, rd, rs, 15, addr, 2);
>                  }
>                  tcg_temp_free_i32(addr);
> -            } else if ((insn & (1 << 6)) == 0) {
> +            } else if ((insn & (7 << 5)) == 0) {
>                  /* Table Branch.  */
>                  if (rn == 15) {
>                      addr = tcg_temp_new_i32();

The thing is that this change on its own is just shifting
the patterns that should UNDEF from the 'table branch'
arm of the elseif ladder into the 'load/store exclusive'
arm, and the latter doesn't (yet) have enough decode to
throw out the invalid cases. That's why I thought it made
more sense as part of the LDA/STL patch, because you
have to update the decode of the load/store excl arm of
the ladder anyway.

If you want this as a separate patch then it should include
the necessary bits of decode in the ld/st excl arm to
cause the patterns to undef (then the lda/stl patch
can fill in extra cases in the switch() that this
patch would introduce.)

thanks
-- PMM

Peter Maydell <peter.maydell@linaro.org> writes:

> On 17 June 2013 14:00, Mans Rullgard <mans@mansr.com> wrote:
>> This avoids other opcodes being incorrectly decoded as TBB/TBH.
>> The LDA/STL instructions new in ARMv8 use this space.
>>
>> Signed-off-by: Mans Rullgard <mans@mansr.com>
>> ---
>> This was previously sent as part of the LDA/STL patch.  Separating it
>> seems clearer.
>> ---
>>  target-arm/translate.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target-arm/translate.c b/target-arm/translate.c
>> index 3ffe7b8..bc41f7e 100644
>> --- a/target-arm/translate.c
>> +++ b/target-arm/translate.c
>> @@ -8126,7 +8126,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
>>                      gen_store_exclusive(s, rd, rs, 15, addr, 2);
>>                  }
>>                  tcg_temp_free_i32(addr);
>> -            } else if ((insn & (1 << 6)) == 0) {
>> +            } else if ((insn & (7 << 5)) == 0) {
>>                  /* Table Branch.  */
>>                  if (rn == 15) {
>>                      addr = tcg_temp_new_i32();
>
> The thing is that this change on its own is just shifting
> the patterns that should UNDEF from the 'table branch'
> arm of the elseif ladder into the 'load/store exclusive'
> arm, and the latter doesn't (yet) have enough decode to
> throw out the invalid cases. That's why I thought it made
> more sense as part of the LDA/STL patch, because you
> have to update the decode of the load/store excl arm of
> the ladder anyway.
>
> If you want this as a separate patch then it should include
> the necessary bits of decode in the ld/st excl arm to
> cause the patterns to undef (then the lda/stl patch
> can fill in extra cases in the switch() that this
> patch would introduce.)

You're quite right.  Keeping that one line in the big patch is probably
the simplest after all.