On 11/19/20 3:56 PM, Peter Maydell wrote:
> Implement the v8.1M FPCXT_NS floating-point system register. This is
> a little more complicated than FPCXT_S, because it has specific
> handling for "current FP state is inactive", and it only wants to do
> PreserveFPState(), not the full set of actions done by
> ExecuteFPCheck() which vfp_access_check() implements.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> target/arm/translate-vfp.c.inc | 110 ++++++++++++++++++++++++++++++---
> 1 file changed, 103 insertions(+), 7 deletions(-)
>
> diff --git a/target/arm/translate-vfp.c.inc b/target/arm/translate-vfp.c.inc
> index ebc59daf613..1c2d31f6f30 100644
> --- a/target/arm/translate-vfp.c.inc
> +++ b/target/arm/translate-vfp.c.inc
> @@ -647,8 +647,20 @@ typedef enum fp_sysreg_check_result {
> fp_sysreg_check_continue, /* caller should continue generating code */
> } fp_sysreg_check_result;
>
> -static fp_sysreg_check_result fp_sysreg_checks(DisasContext *s, int regno)
> +/*
> + * Emit code to check common UNDEF cases and handle lazy state preservation
> + * including the special casing for FPCXT_NS. For reads of sysregs, caller
> + * should provide storefn and opaque; for writes to sysregs these can be
> NULL.
> + * On return, if *insn_end_label is not NULL the caller needs to
> gen_set_label()
> + * it at the end of the other code generated for the insn.
> + */
> +static fp_sysreg_check_result fp_sysreg_checks(DisasContext *s, int regno,
> + fp_sysreg_storefn *storefn,
> + void *opaque,
> + TCGLabel **insn_end_label)
I think it is really ugly to bury the fpAccess check in here.
> - if (!vfp_access_check(s)) {
> - return fp_sysreg_check_done;
I think it would be better to do
if (regno != ARM_VFP_FPCXT_NS && !vfp_access_check(s))
and split out
> + /* fpInactive = FPCCR_NS.ASPEN == 1 && CONTROL.FPCA == 0 */
> + TCGLabel *fp_active_label = gen_new_label();
> + TCGv_i32 aspen, fpca;
> + aspen = load_cpu_field(v7m.fpccr[M_REG_NS]);
> + fpca = load_cpu_field(v7m.control[M_REG_S]);
> + tcg_gen_andi_i32(aspen, aspen, R_V7M_FPCCR_ASPEN_MASK);
> + tcg_gen_xori_i32(aspen, aspen, R_V7M_FPCCR_ASPEN_MASK);
> + tcg_gen_andi_i32(fpca, fpca, R_V7M_CONTROL_FPCA_MASK);
> + tcg_gen_or_i32(fpca, fpca, aspen);
> + tcg_gen_brcondi_i32(TCG_COND_NE, fpca, 0, fp_active_label);
> + tcg_temp_free_i32(aspen);
> + tcg_temp_free_i32(fpca);
> +
> + /* fpInactive case: FPCXT_NS reads as FPDSCR_NS, write is NOP */
> + if (storefn) {
> + TCGv_i32 tmp = load_cpu_field(v7m.fpdscr[M_REG_NS]);
> + storefn(s, opaque, tmp);
> + }
> + /* jump to end of insn */
> + *insn_end_label = gen_new_label();
> + tcg_gen_br(*insn_end_label);
> +
> + gen_set_label(fp_active_label);
> + /* !fpInactive: PreserveFPState() and handle register as normal */
> + gen_preserve_fp_state(s);
... all of this to a separate function.
Then VLDR becomes
case ARM_VFP_FPCXT_NS:
lab_inactive = gen_new_label();
gen_branch_fpInactive(s, TCG_COND_NE, lab_inactive);
...
gen_set_label(lab_inactive);
gen_lookup_tb();
break;
and VSTR becomes
case ARM_VFP_FPCXT_NS:
lab_end = gen_new_label();
lab_active = gen_new_label();
gen_branch_fpInactive(s, TCG_COND_EQ, lab_active);
...
tcg_gen_br(lab_end);
gen_set_label(lab_active);
/* fall through */
case ARM_VFP_FPCXT_S:
...
}
if (lab_end) {
gen_set_label(lab_end);
}
r~
