On 7/5/19 9:24 AM, Laurent Desnogues wrote:
> Hello,
>
> On Tue, Jul 2, 2019 at 4:18 AM Peter Maydell <peter.mayd...@linaro.org> wrote:
>>
>> From: Philippe Mathieu-Daudé <phi...@redhat.com>
>>
>> The vfp_set_fpscr() helper contains code specific to the host
>> floating point implementation (here the SoftFloat library).
>> Extract this code to vfp_set_fpscr_to_host().
>>
>> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>> Message-id: 20190701132516.26392-16-phi...@redhat.com
>> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
>> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
>> ---
>> target/arm/vfp_helper.c | 127 +++++++++++++++++++++-------------------
>> 1 file changed, 66 insertions(+), 61 deletions(-)
>>
>> diff --git a/target/arm/vfp_helper.c b/target/arm/vfp_helper.c
>> index d54e3253240..b19a395b67d 100644
>> --- a/target/arm/vfp_helper.c
>> +++ b/target/arm/vfp_helper.c
>> @@ -81,71 +81,11 @@ static inline int vfp_exceptbits_to_host(int target_bits)
>> return host_bits;
>> }
>>
>> -uint32_t HELPER(vfp_get_fpscr)(CPUARMState *env)
>> -{
>> - uint32_t i, fpscr;
>> -
>> - fpscr = env->vfp.xregs[ARM_VFP_FPSCR]
>> - | (env->vfp.vec_len << 16)
>> - | (env->vfp.vec_stride << 20);
>> -
>> - i = get_float_exception_flags(&env->vfp.fp_status);
>> - i |= get_float_exception_flags(&env->vfp.standard_fp_status);
>> - /* FZ16 does not generate an input denormal exception. */
>> - i |= (get_float_exception_flags(&env->vfp.fp_status_f16)
>> - & ~float_flag_input_denormal);
>> - fpscr |= vfp_exceptbits_from_host(i);
>> -
>> - i = env->vfp.qc[0] | env->vfp.qc[1] | env->vfp.qc[2] | env->vfp.qc[3];
>> - fpscr |= i ? FPCR_QC : 0;
>> -
>> - return fpscr;
>> -}
>> -
>> -uint32_t vfp_get_fpscr(CPUARMState *env)
>> -{
>> - return HELPER(vfp_get_fpscr)(env);
>> -}
>> -
>> -void HELPER(vfp_set_fpscr)(CPUARMState *env, uint32_t val)
>> +static void vfp_set_fpscr_to_host(CPUARMState *env, uint32_t val)
>> {
>> int i;
>> uint32_t changed = env->vfp.xregs[ARM_VFP_FPSCR];
>>
>> - /* When ARMv8.2-FP16 is not supported, FZ16 is RES0. */
>> - if (!cpu_isar_feature(aa64_fp16, env_archcpu(env))) {
>> - val &= ~FPCR_FZ16;
>> - }
>> -
>> - if (arm_feature(env, ARM_FEATURE_M)) {
>> - /*
>> - * M profile FPSCR is RES0 for the QC, STRIDE, FZ16, LEN bits
>> - * and also for the trapped-exception-handling bits IxE.
>> - */
>> - val &= 0xf7c0009f;
>> - }
>> -
>> - /*
>> - * We don't implement trapped exception handling, so the
>> - * trap enable bits, IDE|IXE|UFE|OFE|DZE|IOE are all RAZ/WI (not RES0!)
>> - *
>> - * If we exclude the exception flags, IOC|DZC|OFC|UFC|IXC|IDC
>> - * (which are stored in fp_status), and the other RES0 bits
>> - * in between, then we clear all of the low 16 bits.
>> - */
>> - env->vfp.xregs[ARM_VFP_FPSCR] = val & 0xf7c80000;
>> - env->vfp.vec_len = (val >> 16) & 7;
>> - env->vfp.vec_stride = (val >> 20) & 3;
>> -
>> - /*
>> - * The bit we set within fpscr_q is arbitrary; the register as a
>> - * whole being zero/non-zero is what counts.
>> - */
>> - env->vfp.qc[0] = val & FPCR_QC;
>> - env->vfp.qc[1] = 0;
>> - env->vfp.qc[2] = 0;
>> - env->vfp.qc[3] = 0;
>> -
>> changed ^= val;
>> if (changed & (3 << 22)) {
>> i = (val >> 22) & 3;
>> @@ -193,6 +133,71 @@ void HELPER(vfp_set_fpscr)(CPUARMState *env, uint32_t
>> val)
>> set_float_exception_flags(0, &env->vfp.standard_fp_status);
>> }
>>
>> +uint32_t HELPER(vfp_get_fpscr)(CPUARMState *env)
>> +{
>> + uint32_t i, fpscr;
>> +
>> + fpscr = env->vfp.xregs[ARM_VFP_FPSCR]
>> + | (env->vfp.vec_len << 16)
>> + | (env->vfp.vec_stride << 20);
>> +
>> + i = get_float_exception_flags(&env->vfp.fp_status);
>> + i |= get_float_exception_flags(&env->vfp.standard_fp_status);
>> + /* FZ16 does not generate an input denormal exception. */
>> + i |= (get_float_exception_flags(&env->vfp.fp_status_f16)
>> + & ~float_flag_input_denormal);
>> + fpscr |= vfp_exceptbits_from_host(i);
>> +
>> + i = env->vfp.qc[0] | env->vfp.qc[1] | env->vfp.qc[2] | env->vfp.qc[3];
>> + fpscr |= i ? FPCR_QC : 0;
>> +
>> + return fpscr;
>> +}
>> +
>> +uint32_t vfp_get_fpscr(CPUARMState *env)
>> +{
>> + return HELPER(vfp_get_fpscr)(env);
>> +}
>> +
>> +void HELPER(vfp_set_fpscr)(CPUARMState *env, uint32_t val)
>> +{
>> + /* When ARMv8.2-FP16 is not supported, FZ16 is RES0. */
>> + if (!cpu_isar_feature(aa64_fp16, env_archcpu(env))) {
>> + val &= ~FPCR_FZ16;
>> + }
>> +
>> + if (arm_feature(env, ARM_FEATURE_M)) {
>> + /*
>> + * M profile FPSCR is RES0 for the QC, STRIDE, FZ16, LEN bits
>> + * and also for the trapped-exception-handling bits IxE.
>> + */
>> + val &= 0xf7c0009f;
>> + }
>> +
>> + /*
>> + * We don't implement trapped exception handling, so the
>> + * trap enable bits, IDE|IXE|UFE|OFE|DZE|IOE are all RAZ/WI (not RES0!)
>> + *
>> + * If we exclude the exception flags, IOC|DZC|OFC|UFC|IXC|IDC
>> + * (which are stored in fp_status), and the other RES0 bits
>> + * in between, then we clear all of the low 16 bits.
>> + */
>> + env->vfp.xregs[ARM_VFP_FPSCR] = val & 0xf7c80000;
>> + env->vfp.vec_len = (val >> 16) & 7;
>> + env->vfp.vec_stride = (val >> 20) & 3;
>> +
>> + /*
>> + * The bit we set within fpscr_q is arbitrary; the register as a
>> + * whole being zero/non-zero is what counts.
>> + */
>> + env->vfp.qc[0] = val & FPCR_QC;
>> + env->vfp.qc[1] = 0;
>> + env->vfp.qc[2] = 0;
>> + env->vfp.qc[3] = 0;
>> +
>> + vfp_set_fpscr_to_host(env, val);
>> +}
>> +
>> void vfp_set_fpscr(CPUARMState *env, uint32_t val)
>> {
>> HELPER(vfp_set_fpscr)(env, val);
>
> This patch breaks flag settings because at the point where
> vfp_set_fpscr_to_host is called, the value in
> env->vfp.xregs[ARM_VFP_FPSCR] has already been changed.
Oops.
> A possible fix to that issue to is to save FPCR value when entering
> the helper and passing it to vfp_set_fpscr_to_host.
OK, thanks,
Phil.
