Re: [PATCH v6 11/20] target/arm: Hoist computation of TBFLAG_A32.VFPEN

[Alex Bennée]

Richard Henderson <address@hidden> writes:

> There are 3 conditions that each enable this flag.  M-profile always
> enables; A-profile with EL1 as AA64 always enables.  Both of these
> conditions can easily be cached.  The final condition relies on the
> FPEXC register which we are not prepared to cache.
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/cpu.h    |  2 +-
>  target/arm/helper.c | 14 ++++++++++----
>  2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 4d961474ce..9909ff89d4 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -3192,7 +3192,7 @@ FIELD(TBFLAG_A32, XSCALE_CPAR, 4, 2)
>   * the same thing as the current security state of the processor!
>   */
>  FIELD(TBFLAG_A32, NS, 6, 1)
> -FIELD(TBFLAG_A32, VFPEN, 7, 1)          /* Not cached. */
> +FIELD(TBFLAG_A32, VFPEN, 7, 1)          /* Partially cached, minus FPEXC. */
>  FIELD(TBFLAG_A32, CONDEXEC, 8, 8)       /* Not cached. */
>  FIELD(TBFLAG_A32, SCTLR_B, 16, 1)
>  /* For M profile only, set if FPCCR.LSPACT is set */
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 398e5f5d6d..89aa6fd933 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -11088,6 +11088,9 @@ static uint32_t rebuild_hflags_m32(CPUARMState *env, 
> int fp_el,
>  {
>      uint32_t flags = 0;
>
> +    /* v8M always enables the fpu.  */
> +    flags = FIELD_DP32(flags, TBFLAG_A32, VFPEN, 1);
> +
>      if (arm_v7m_is_handler_mode(env)) {
>          flags = FIELD_DP32(flags, TBFLAG_A32, HANDLER, 1);
>      }
> @@ -11119,6 +11122,10 @@ static uint32_t rebuild_hflags_a32(CPUARMState *env, 
> int fp_el,
>                                     ARMMMUIdx mmu_idx)
>  {
>      uint32_t flags = rebuild_hflags_aprofile(env);
> +
> +    if (arm_el_is_aa64(env, 1)) {
> +        flags = FIELD_DP32(flags, TBFLAG_A32, VFPEN, 1);
> +    }
>      return rebuild_hflags_common_32(env, fp_el, mmu_idx, flags);
>  }
>
> @@ -11250,14 +11257,13 @@ void cpu_get_tb_cpu_state(CPUARMState *env, 
> target_ulong *pc,
>                  flags = FIELD_DP32(flags, TBFLAG_A32, VECSTRIDE,
>                                     env->vfp.vec_stride);
>              }
> +            if (env->vfp.xregs[ARM_VFP_FPEXC] & (1 << 30)) {
> +                flags = FIELD_DP32(flags, TBFLAG_A32, VFPEN, 1);
> +            }

We seem to be short of symbolic definitions for this bit but whatever:

Reviewed-by: Alex Bennée <address@hidden>

>          }
>
>          flags = FIELD_DP32(flags, TBFLAG_A32, THUMB, env->thumb);
>          flags = FIELD_DP32(flags, TBFLAG_A32, CONDEXEC, env->condexec_bits);
> -        if (env->vfp.xregs[ARM_VFP_FPEXC] & (1 << 30)
> -            || arm_el_is_aa64(env, 1) || arm_feature(env, ARM_FEATURE_M)) {
> -            flags = FIELD_DP32(flags, TBFLAG_A32, VFPEN, 1);
> -        }
>          pstate_for_ss = env->uncached_cpsr;
>      }


--
Alex Bennée