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

[Richard Henderson]

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~