Re: [Qemu-devel] [PATCH] target-arm: decode TBB/TBH more thoroughly

[Måns Rullgård]

Peter Maydell <address@hidden> writes:

> On 17 June 2013 14:00, Mans Rullgard <address@hidden> 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 <address@hidden>
>> ---
>> 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.

-- 
Måns Rullgård
address@hidden