Re: [Qemu-devel] [PATCH for-2.5 20/30] m68k: add exg

[Laurent Vivier]

Le 13/08/2015 01:09, Richard Henderson a écrit :
> On 08/12/2015 03:43 PM, Laurent Vivier wrote:
>>
>>
>> Le 12/08/2015 19:05, Richard Henderson a écrit :
>>> On 08/09/2015 01:13 PM, Laurent Vivier wrote:
>>>> Signed-off-by: Laurent Vivier <address@hidden>
>>>> ---
>>>>   target-m68k/translate.c | 34 +++++++++++++++++++++++++++++++++-
>>>>   1 file changed, 33 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/target-m68k/translate.c b/target-m68k/translate.c
>>>> index adf4521..b7d15e9 100644
>>>> --- a/target-m68k/translate.c
>>>> +++ b/target-m68k/translate.c
>>>> @@ -2035,10 +2035,42 @@ DISAS_INSN(and)
>>>>       TCGv dest;
>>>>       TCGv addr;
>>>>       int opsize;
>>>> +    int exg_mode;
>>>>
>>>> +    dest = tcg_temp_new();
>>>> +
>>>> +    /* exg */
>>>> +
>>>> +    exg_mode = insn & 0x1f8;
>>>
>>> Likewise, surely we can decode EXG separately from AND, and avoid
>>> doing so for
>>> coldfire.
>>
>> I agree for CMPM, not for EXG.
>>
>> Let's have a look to instructions encoding :)
>>
>> AND       1100dddooommmrrr
>>
>>    ddd       data register number
>>    ooo       opmode,  invalid: 011, 111
>>    mmmrrr    ea mode, if ooo = { 000, 001, 010} invalid: 001000 .. 001111
>>                       if ooo = { 100, 101, 110} invalid; 000000 .. 001111
>>
>> EXG       1100xxx1oooooyyy
>>
>>    xxx       register
>>    ooooo     valid: 01000, 01001, 10001
>>    yyy       register
>>
>> So, EXG is an AND with
>>
>>    ooo 101, 110
>>    mmm 000, 001
>>
>> which are invalid combinations for AND.
>>
>> IMHO, EXG looks like a wart on the AND and should be decoded like that...
> 
> Hmm, perhaps you're right.  On the other hand, maybe we should rename
> the function and_exg, and also properly check for M68000 before
> accepting exg?

I agree.

Thank you for all your comments.
Laurent