Re: [PATCH v5 10/17] target/m68k: Implement TRAPcc

[Richard Henderson <richard.henderson@linaro.org>]

On 5/31/22 01:01, Laurent Vivier wrote:
> Le 27/05/2022 à 18:48, Richard Henderson a écrit :
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/754
>> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/m68k/cpu.h          |  2 ++
>>   linux-user/m68k/cpu_loop.c |  1 +
>>   target/m68k/cpu.c          |  1 +
>>   target/m68k/op_helper.c    |  6 +----
>>   target/m68k/translate.c    | 49 ++++++++++++++++++++++++++++++++++++++
>>   5 files changed, 54 insertions(+), 5 deletions(-)
>>
> ...
>> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
>> index 399d9232e4..c4fe8abc03 100644
>> --- a/target/m68k/translate.c
>> +++ b/target/m68k/translate.c
> ...
>> @@ -6050,6 +6098,7 @@ void register_m68k_insns (CPUM68KState *env)
>>       INSN(scc,       50c0, f0f8, CF_ISA_A); /* Scc.B Dx   */
>>       INSN(scc,       50c0, f0c0, M68000);   /* Scc.B <EA> */
>>       INSN(dbcc,      50c8, f0f8, M68000);
>> +    INSN(trapcc,    50f8, f0f8, TRAPCC);
>>       INSN(tpf,       51f8, fff8, CF_ISA_A);
>>       /* Branch instructions.  */
> 
> This one breaks Mark's series to support MacOS.
> 
> I think the new opcode short-circuits Scc one:
> 
>    ----------------
>    IN: INITRSRCMGR
>    0x408011d0:  st 0xa58
>    Disassembler disagrees with translator over instruction decoding
>    Please report this to qemu-devel@nongnu.org
> 
> The following patch seems to fix the problem:
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index d5d73401b7cc..3b0e3d0b58f6 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -6119,9 +6119,9 @@ void register_m68k_insns (CPUM68KState *env)
>       INSN(addsubq,   5000, f080, M68000);
>       BASE(addsubq,   5080, f0c0);
>       INSN(scc,       50c0, f0f8, CF_ISA_A); /* Scc.B Dx   */
> +    INSN(trapcc,    50f8, f0f8, TRAPCC);
>       INSN(scc,       50c0, f0c0, M68000);   /* Scc.B <EA> */
>       INSN(dbcc,      50c8, f0f8, M68000);
> -    INSN(trapcc,    50f8, f0f8, TRAPCC);

Hmm.  That will completely hide trapcc -- you should have seen the new test case fail (and 
if not, the test case needs fixing).

These two insn overlap considerably:

    setcc  0101 cond:4 11 mode:3 reg:3
    trapcc 0101 cond:4 11 111 opmode:3

We need to select only the 3 valid opmodes:

     INSN(scc,       50c0, f0c0, M68000);   /* Scc.B <EA> */
     INSN(dbcc,      50c8, f0f8, M68000);
     INSN(trapcc,    50fa, f0fe, TRAPCC);   /* opmode 010, 011 */
     INSN(trapcc,    50fc, f0ff, TRAPCC);   /* opmode 100 */
     INSN(trapcc,    51fa, fffe, CP_ISA_A); /* TPF (trapf) opmode 010, 011 */
     INSN(trapcc,    51fc, ffff, CP_ISA_A); /* TPF (trapf) opmode 100 */

which are invalid mode/reg combinations for Scc.


r~