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

[Jia Liu]

Hi Blue,

On Sat, May 19, 2012 at 4:29 PM, Blue Swirl <address@hidden> wrote:
> 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.

fixed.

>
>> +{
>> +    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.
>

there is already a exception handle in helpers.

>>         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
>>
>>


Regards,
Jia.