Re: [Qemu-devel] [PATCH v2 05/32] target/arm/cpu.h: add additional float_status flags

[Richard Henderson]

On 02/08/2018 09:31 AM, Alex Bennée wrote:
> @@ -10750,15 +10751,22 @@ void HELPER(vfp_set_fpscr)(CPUARMState *env, 
> uint32_t val)
>  }
>  set_float_rounding_mode(i, >vfp.fp_status);
>  }
> -if (changed & (1 << 24)) {
> +if (changed & (1 << 19)) { /* FPCR:FZ16 */
> +set_flush_to_zero((val & (1 << 19)) != 0, >vfp.fp_status_f16);
> +set_flush_inputs_to_zero((val & (1 << 19)) != 0,
> + >vfp.fp_status_f16);
> +}
> +if (changed & (1 << 24)) { /* FPCR:FZ */
>  set_flush_to_zero((val & (1 << 24)) != 0, >vfp.fp_status);
>  set_flush_inputs_to_zero((val & (1 << 24)) != 0, 
> >vfp.fp_status);
>  }
> -if (changed & (1 << 25))
> +if (changed & (1 << 25)) { /* FPCR:DN */
>  set_default_nan_mode((val & (1 << 25)) != 0, >vfp.fp_status);
> +}

This misses changing rounding mode and default_nan_mode for fp_status_f16.

>  set_float_exception_flags(i, >vfp.fp_status);
> +set_float_exception_flags(i, >vfp.fp_status_f16);
>  set_float_exception_flags(0, >vfp.standard_fp_status);

I think you should store 0 into fp_status_f16 here.  The exception flags are
OR'ed together when reading; there's not too much point in replicating it out.


r~

[Qemu-devel] [PATCH v2 05/32] target/arm/cpu.h: add additional float_status flags

[Alex Bennée]

Half-precision flush to zero behaviour is controlled by a separate
FZ16 bit in the FPCR. To handle this we pass a pointer to
fp_status_fp16 when working on half-precision operations. The value of
the presented FPCR is calculated from an amalgam of the two when read.

Signed-off-by: Alex Bennée 
---
 target/arm/cpu.h   | 22 +--
 target/arm/helper.c| 12 +--
 target/arm/translate-a64.c | 53 +-
 3 files changed, 55 insertions(+), 32 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index f976969011..97c9352a0f 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -501,19 +501,29 @@ typedef struct CPUARMState {
 /* scratch space when Tn are not sufficient.  */
 uint32_t scratch[8];
 
-/* fp_status is the "normal" fp status. standard_fp_status retains
- * values corresponding to the ARM "Standard FPSCR Value", ie
- * default-NaN, flush-to-zero, round-to-nearest and is used by
- * any operations (generally Neon) which the architecture defines
- * as controlled by the standard FPSCR value rather than the FPSCR.
+/* There are a number of distinct float control structures:
+ *
+ *  fp_status: is the "normal" fp status.
+ *  fp_status_fp16: used for half-precision calculations
+ *  standard_fp_status : the ARM "Standard FPSCR Value"
+ *
+ * Half-precision operations are governed by a separate
+ * flush-to-zero control bit in FPSCR:FZ16. We pass a separate
+ * status structure to control this.
+ *
+ * The "Standard FPSCR", ie default-NaN, flush-to-zero,
+ * round-to-nearest and is used by any operations (generally
+ * Neon) which the architecture defines as controlled by the
+ * standard FPSCR value rather than the FPSCR.
  *
  * To avoid having to transfer exception bits around, we simply
  * say that the FPSCR cumulative exception flags are the logical
- * OR of the flags in the two fp statuses. This relies on the
+ * OR of the flags in the three fp statuses. This relies on the
  * only thing which needs to read the exception flags being
  * an explicit FPSCR read.
  */
 float_status fp_status;
+float_status fp_status_f16;
 float_status standard_fp_status;
 } vfp;
 uint64_t exclusive_addr;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 4ef99882c4..1cc3d43a9f 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10692,6 +10692,7 @@ uint32_t HELPER(vfp_get_fpscr)(CPUARMState *env)
 | (env->vfp.vec_stride << 20);
 i = get_float_exception_flags(>vfp.fp_status);
 i |= get_float_exception_flags(>vfp.standard_fp_status);
+i |= get_float_exception_flags(>vfp.fp_status_f16);
 fpscr |= vfp_exceptbits_from_host(i);
 return fpscr;
 }
@@ -10750,15 +10751,22 @@ void HELPER(vfp_set_fpscr)(CPUARMState *env, uint32_t 
val)
 }
 set_float_rounding_mode(i, >vfp.fp_status);
 }
-if (changed & (1 << 24)) {
+if (changed & (1 << 19)) { /* FPCR:FZ16 */
+set_flush_to_zero((val & (1 << 19)) != 0, >vfp.fp_status_f16);
+set_flush_inputs_to_zero((val & (1 << 19)) != 0,
+ >vfp.fp_status_f16);
+}
+if (changed & (1 << 24)) { /* FPCR:FZ */
 set_flush_to_zero((val & (1 << 24)) != 0, >vfp.fp_status);
 set_flush_inputs_to_zero((val & (1 << 24)) != 0, >vfp.fp_status);
 }
-if (changed & (1 << 25))
+if (changed & (1 << 25)) { /* FPCR:DN */
 set_default_nan_mode((val & (1 << 25)) != 0, >vfp.fp_status);
+}
 
 i = vfp_exceptbits_to_host(val);
 set_float_exception_flags(i, >vfp.fp_status);
+set_float_exception_flags(i, >vfp.fp_status_f16);
 set_float_exception_flags(0, >vfp.standard_fp_status);
 }
 
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index eed64c73e5..1afa669e6e 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -604,16 +604,21 @@ static void write_fp_sreg(DisasContext *s, int reg, 
TCGv_i32 v)
 tcg_temp_free_i64(tmp);
 }
 
-static TCGv_ptr get_fpstatus_ptr(void)
+static TCGv_ptr get_fpstatus_ptr(bool is_f16)
 {
 TCGv_ptr statusptr = tcg_temp_new_ptr();
 int offset;
 
-/* In A64 all instructions (both FP and Neon) use the FPCR;
- * there is no equivalent of the A32 Neon "standard FPSCR value"
- * and all operations use vfp.fp_status.
+/* In A64 all instructions (both FP and Neon) use the FPCR; there
+ * is no equivalent of the A32 Neon "standard FPSCR value".
+ * However half-precision operations operate under a different
+ * FZ16 flag and use vfp.fp_status_f16 instead of vfp.fp_status.
  */
-offset = offsetof(CPUARMState, vfp.fp_status);
+if (is_f16) {
+offset =