Re: [Qemu-devel] [PATCH 07/15] Openrisc: add float instruction helpers

[Blue Swirl]

On Thu, May 17, 2012 at 8:35 AM, Jia Liu <address@hidden> wrote:
> add the openrisc float instruction helpers.
>
> Signed-off-by: Jia Liu <address@hidden>
> ---
>  Makefile.target              |    2 +-
>  target-openrisc/fpu_helper.c |   93 
> ++++++++++++++++++++++++++++++++++++++++++
>  target-openrisc/helper.h     |    6 +++
>  target-openrisc/translate.c  |    8 ++--
>  4 files changed, 104 insertions(+), 5 deletions(-)
>  create mode 100644 target-openrisc/fpu_helper.c
>
> diff --git a/Makefile.target b/Makefile.target
> index 9bebdb3..8a7b743 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -101,7 +101,7 @@ endif
>  libobj-$(TARGET_SPARC) += int32_helper.o
>  libobj-$(TARGET_SPARC64) += int64_helper.o
>  libobj-$(TARGET_ALPHA) += int_helper.o fpu_helper.o sys_helper.o mem_helper.o
> -libobj-$(TARGET_OPENRISC) += excp.o excp_helper.o int_helper.o 
> intrp_helper.o mem.o mem_helper.o
> +libobj-$(TARGET_OPENRISC) += excp.o excp_helper.o fpu_helper.o int_helper.o 
> intrp_helper.o mem.o mem_helper.o
>
>  libobj-y += disas.o
>  libobj-$(CONFIG_TCI_DIS) += tci-dis.o
> diff --git a/target-openrisc/fpu_helper.c b/target-openrisc/fpu_helper.c
> new file mode 100644
> index 0000000..be2bfb5
> --- /dev/null
> +++ b/target-openrisc/fpu_helper.c
> @@ -0,0 +1,93 @@
> +/*
> + * OpenRISC float helper routines
> + *
> + *  Copyright (c) 2011-2012 Jia Liu <address@hidden>
> + *                          Feng Gao <address@hidden>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Library General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Library General Public License for more details.
> + *
> + * You should have received a copy of the GNU Library General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA  02110-1301 
> USA
> + */
> +
> +#include "cpu.h"
> +#include "helper.h"
> +#include "excp.h"
> +
> +static inline int ieee_ex_to_openrisc(int xcpt)
> +{
> +    int ret = 0;
> +
> +    if (xcpt) {
> +        if (xcpt & float_flag_invalid) {
> +            ret &= FPCSR_FPEE;
> +        }
> +        if (xcpt & float_flag_overflow) {
> +            ret |= FPCSR_OVF;
> +        }
> +        if (xcpt & float_flag_underflow) {
> +            ret |= FPCSR_UNF;
> +        }
> +        if (xcpt & float_flag_divbyzero) {
> +            ret |= FPCSR_DZF;
> +        }
> +    }
> +
> +    return ret;
> +}
> +
> +static inline void update_fpcsr(CPUOPENRISCState *env)
> +{
> +    int tmp = 
> ieee_ex_to_openrisc(get_float_exception_flags(&env->fp_status));

A blank line after variable declarations would be nice.

> +    SET_FP_CAUSE(env->fpcsr, tmp);
> +    if (GET_FP_ENABLE(env->fpcsr) & tmp) {
> +        helper_exception(env, EXCP_FPE);
> +    } else {
> +      UPDATE_FP_FLAGS(env->fpcsr, tmp);
> +    }
> +}
> +
> +target_ulong HELPER(itofd)(CPUOPENRISCState *env, target_ulong val)

This will truncate the 64 bit value to 32 bits if that is the size of
target_ulong, I don't think that is correct.

> +{
> +    uint64_t itofd;
> +    set_float_exception_flags(0, &env->fp_status);
> +    itofd = int32_to_float64(val, &env->fp_status);
> +    update_fpcsr(env);
> +    return itofd;
> +}
> +
> +target_ulong HELPER(itofs)(CPUOPENRISCState *env, target_ulong val)
> +{
> +    target_ulong itofs;
> +    set_float_exception_flags(0, &env->fp_status);
> +    itofs = int32_to_float32(val, &env->fp_status);
> +    update_fpcsr(env);
> +    return itofs;
> +}
> +
> +target_ulong HELPER(ftoid)(CPUOPENRISCState *env, target_ulong val)

Ditto.

> +{
> +    target_ulong ftoid;
> +    set_float_exception_flags(0, &env->fp_status);
> +    ftoid = float32_to_int64(val, &env->fp_status);
> +    update_fpcsr(env);
> +    return ftoid;
> +}
> +
> +target_ulong HELPER(ftois)(CPUOPENRISCState *env, target_ulong val)
> +{
> +    target_ulong ftois;
> +    set_float_exception_flags(0, &env->fp_status);
> +    ftois = float32_to_int32(val, &env->fp_status);
> +    update_fpcsr(env);
> +    return ftois;
> +}
> diff --git a/target-openrisc/helper.h b/target-openrisc/helper.h
> index f5ed117..b9b1d67 100644
> --- a/target-openrisc/helper.h
> +++ b/target-openrisc/helper.h
> @@ -23,6 +23,12 @@
>  /* exception */
>  DEF_HELPER_FLAGS_2(exception, 0, void, env, i32)
>
> +/* float */
> +DEF_HELPER_FLAGS_2(itofd, 0, tl, env, tl)
> +DEF_HELPER_FLAGS_2(itofs, 0, tl, env, tl)
> +DEF_HELPER_FLAGS_2(ftoid, 0, tl, env, tl)
> +DEF_HELPER_FLAGS_2(ftois, 0, tl, env, tl)
> +
>  /* int */
>  DEF_HELPER_FLAGS_1(ff1, 0, tl, tl)
>  DEF_HELPER_FLAGS_1(fl1, 0, tl, tl)
> diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c
> index dffc8a7..7d942b5 100644
> --- a/target-openrisc/translate.c
> +++ b/target-openrisc/translate.c
> @@ -1107,22 +1107,22 @@ static void dec_float(DisasContext *dc, 
> CPUOPENRISCState *env, uint32_t insn)
>
>     case 0x14:    /*lf.itof.d*/
>         LOG_DIS("lf.itof r%d, r%d\n", rd, ra);
> -        /* itof.d need a helper here */
> +        gen_helper_itofd(cpu_R[rd], cpu_env, cpu_R[ra]);

You probably need to save PC etc. if this instruction can cause
exceptions, otherwise return PC will not be correct and guest
exception handler will return to start of the block.

>         break;
>
>     case 0x04:    /*lf.itof.s*/
>         LOG_DIS("lf.itof r%d, r%d\n", rd, ra);
> -        /* itof.s need a helper here */
> +        gen_helper_itofs(cpu_R[rd], cpu_env, cpu_R[ra]);
>         break;
>
>     case 0x15:    /*lf.ftoi.d*/
>         LOG_DIS("lf.ftoi r%d, r%d\n", rd, ra);
> -        /* ftoi.d need a helper here */
> +        gen_helper_ftoid(cpu_R[rd], cpu_env, cpu_R[ra]);
>         break;
>
>     case 0x05:    /*lf.ftoi.s*/
>         LOG_DIS("lf.ftoi r%d, r%d\n", rd, ra);
> -        /* ftoi.s need a helper here */
> +        gen_helper_ftois(cpu_R[rd], cpu_env, cpu_R[ra]);
>         break;
>
>     case 0x16:    /*lf.rem.d*/
> --
> 1.7.9.5
>
>