Re: [Qemu-devel] [PATCH v3 09/16] target-m68k: add fmovem

[Richard Henderson]

On 02/07/2017 11:59 AM, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <address@hidden>
> ---
>  target/m68k/fpu_helper.c |  6 +++
>  target/m68k/helper.h     |  1 +
>  target/m68k/translate.c  | 99 +++++++++++++++++++++++++++++++++++-------------
>  3 files changed, 80 insertions(+), 26 deletions(-)
>
> diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
> index 1e68c41..aadfc82 100644
> --- a/target/m68k/fpu_helper.c
> +++ b/target/m68k/fpu_helper.c
> @@ -421,3 +421,9 @@ void HELPER(update_fpstatus)(CPUM68KState *env)
>
>      set_float_exception_flags(flags, &env->fp_status);
>  }
> +
> +void HELPER(fmovem)(CPUM68KState *env, uint32_t opsize,
> +                    uint32_t mode, uint32_t mask)
> +{
> +    fprintf(stderr, "MISSING HELPER fmovem\n");
> +}

Um... no.


> diff --git a/target/m68k/helper.h b/target/m68k/helper.h
> index 072a6d0..58bc273 100644
> --- a/target/m68k/helper.h
> +++ b/target/m68k/helper.h
> @@ -31,6 +31,7 @@ DEF_HELPER_1(cmp_FP0_FP1, void, env)
>  DEF_HELPER_2(set_fpcr, void, env, i32)
>  DEF_HELPER_1(tst_FP0, void, env)
>  DEF_HELPER_1(update_fpstatus, void, env)
> +DEF_HELPER_4(fmovem, void, env, i32, i32, i32)
>
>  DEF_HELPER_3(mac_move, void, env, i32, i32)
>  DEF_HELPER_3(macmulf, i64, env, i32, i32)
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index f9c64ff..ac60f1a 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -4483,13 +4483,79 @@ static void gen_op_fmove_fcr(CPUM68KState *env, 
> DisasContext *s,
>      tcg_temp_free_i32(addr);
>  }
>
> +static void gen_op_fmovem(CPUM68KState *env, DisasContext *s,
> +                          uint32_t insn, uint32_t ext)
> +{
> +    int opsize;
> +    uint16_t mask;
> +    int i;
> +    uint32_t mode;
> +    int32_t incr;
> +    TCGv addr, tmp;
> +    int is_load;
> +
> +    if (m68k_feature(s->env, M68K_FEATURE_FPU)) {
> +        opsize = OS_EXTENDED;
> +    } else {
> +        opsize = OS_DOUBLE;  /* FIXME */
> +    }
> +
> +    mode = (ext >> 11) & 0x3;
> +    if ((mode & 0x1) == 1) {
> +        gen_helper_fmovem(cpu_env, tcg_const_i32(opsize),
> +                          tcg_const_i32(mode), DREG(ext, 0));

... why not just raise illegal opcode here instead of fprintf.
You should also add a comment about not supporting the dynamic set.

That said... it almost seems easier to support fmovem as a helper than it does 
inline.  So perhaps just always implement it out of line?


r~