Re: [PATCH v3 2/4] target/riscv: Reuse tb->flags.FS

[Richard Henderson <richard.henderson@linaro.org>]

On 4/28/23 17:52, Mayuresh Chitale wrote:
> When misa.F is 0 tb->flags.FS field is unused and can be used to save
> the current state of smstateen0.FCSR check which is needed by the
> floating point translation routines.
> 
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---
>   target/riscv/cpu_helper.c |  9 +++++++++
>   target/riscv/translate.c  | 12 +++++++++++-
>   2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index b68dcfe7b6..126ac221a0 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -119,6 +119,15 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
>           vs = MIN(vs, get_field(env->mstatus_hs, MSTATUS_VS));
>       }
>   
> +    /*
> +     * If misa.F is 0 then the FS field of the tb->flags can be used to pass
> +     * the current state of the smstateen.FCSR bit which must be checked for
> +     * in the floating point translation routines.
> +     */
> +    if (!riscv_has_ext(env, RVF)) {
> +        fs = (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == RISCV_EXCP_NONE);
> +    }

You have misunderstood my suggestion:

     /* With Zfinx, floating point is enabled/disabled by Smstateen. */
     if (!riscv_has_ext(env, RVF)) {
         fs = (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR)
               ? EXT_STATUS_DIRTY : EXT_STATUS_DISABLED);
     }

> +    bool smstateen_fcsr_ok;

Not needed.

> -    ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
> +    if (has_ext(ctx, RVF)) {
> +        ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
> +    } else {
> +        ctx->mstatus_fs = 0;
> +    }

Not needed.

In patch 3, which should be merged with this, there are no changes to REQUIRE_ZFINX_OR_F, 
no additional smstateen_fcsr_check, and REQUIRE_FPU reduces to

#define REQUIRE_FPU do {                          \
     if (ctx->mstatus_fs == EXT_STATUS_DISABLED) { \
         return false;                             \
     }                                             \
} while (0)

This makes the DisasContext version of fs be the single gate for floating point.
No extra checks required.


r~