[PATCH 0/3] target/arm: Fix some FEAT_AFP corner cases

[PATCH 0/3] target/arm: Fix some FEAT_AFP corner cases

[Peter Maydell]

There are a few corner cases where when we implemented FEAT_AFP we
didn't update the instructions to get the FPCR.AH = 1 handling right:
 * SVE2 FMAXP and FMINP need to do the AH=1 style max/min
 * SVE FCVTLT to halfprec needs to use the F16 FPCR
 * FLOGB needs to report when it consumed a denormal

This patchset implements fixes for these.

thanks
-- PMM

Peter Maydell (3):
  target/arm: SVE2 FMAXP, FMINP must honour AH=1
  target/arm: Use FPST_A64_F16 for SVE FCVTLT_hs
  target/arm: Set correct fp flags for FLOGB when FPCR.AH = 1

 target/arm/tcg/helper-sve-defs.h | 14 ++++++++++++++
 target/arm/tcg/sve_helper.c      | 11 +++++++++++
 target/arm/tcg/translate-sve.c   |  6 +++---
 3 files changed, 28 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH 1/3] target/arm: SVE2 FMAXP, FMINP must honour AH=1

[Peter Maydell]

The behaviour of floating-point maximum and minimum insns has
some odd special cases when FPCR.AH=1. We get this right in most
places (for instance, the ASIMD FMAXP, FMINP) but forgot about
it for the SVE2 versions of FMAXP and FMINP.

Cc: qemu-stable@nongnu.org
Fixes: 384433e70983 ("target/arm: Implement FPCR.AH semantics for FMINP and FMAXP")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/tcg/helper-sve-defs.h | 14 ++++++++++++++
 target/arm/tcg/sve_helper.c      |  8 ++++++++
 target/arm/tcg/translate-sve.c   |  4 ++--
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/target/arm/tcg/helper-sve-defs.h b/target/arm/tcg/helper-sve-defs.h
index c3541a8ca8..cd05dd0fb4 100644
--- a/target/arm/tcg/helper-sve-defs.h
+++ b/target/arm/tcg/helper-sve-defs.h
@@ -2914,6 +2914,20 @@ DEF_HELPER_FLAGS_6(sve2_fminp_zpzz_s, TCG_CALL_NO_RWG,
 DEF_HELPER_FLAGS_6(sve2_fminp_zpzz_d, TCG_CALL_NO_RWG,
                    void, ptr, ptr, ptr, ptr, fpst, i32)
 
+DEF_HELPER_FLAGS_6(sve2_ah_fmaxp_zpzz_h, TCG_CALL_NO_RWG,
+                   void, ptr, ptr, ptr, ptr, fpst, i32)
+DEF_HELPER_FLAGS_6(sve2_ah_fmaxp_zpzz_s, TCG_CALL_NO_RWG,
+                   void, ptr, ptr, ptr, ptr, fpst, i32)
+DEF_HELPER_FLAGS_6(sve2_ah_fmaxp_zpzz_d, TCG_CALL_NO_RWG,
+                   void, ptr, ptr, ptr, ptr, fpst, i32)
+
+DEF_HELPER_FLAGS_6(sve2_ah_fminp_zpzz_h, TCG_CALL_NO_RWG,
+                   void, ptr, ptr, ptr, ptr, fpst, i32)
+DEF_HELPER_FLAGS_6(sve2_ah_fminp_zpzz_s, TCG_CALL_NO_RWG,
+                   void, ptr, ptr, ptr, ptr, fpst, i32)
+DEF_HELPER_FLAGS_6(sve2_ah_fminp_zpzz_d, TCG_CALL_NO_RWG,
+                   void, ptr, ptr, ptr, ptr, fpst, i32)
+
 DEF_HELPER_FLAGS_5(sve2_eor3, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
 DEF_HELPER_FLAGS_5(sve2_bcax, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
 DEF_HELPER_FLAGS_5(sve2_bsl1n, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
diff --git a/target/arm/tcg/sve_helper.c b/target/arm/tcg/sve_helper.c
index 062d8881bd..179cbd74fb 100644
--- a/target/arm/tcg/sve_helper.c
+++ b/target/arm/tcg/sve_helper.c
@@ -778,6 +778,14 @@ DO_ZPZZ_PAIR_FP(sve2_fminp_zpzz_h, float16, H1_2, float16_min)
 DO_ZPZZ_PAIR_FP(sve2_fminp_zpzz_s, float32, H1_4, float32_min)
 DO_ZPZZ_PAIR_FP(sve2_fminp_zpzz_d, float64, H1_8, float64_min)
 
+DO_ZPZZ_PAIR_FP(sve2_ah_fmaxp_zpzz_h, float16, H1_2, helper_vfp_ah_maxh)
+DO_ZPZZ_PAIR_FP(sve2_ah_fmaxp_zpzz_s, float32, H1_4, helper_vfp_ah_maxs)
+DO_ZPZZ_PAIR_FP(sve2_ah_fmaxp_zpzz_d, float64, H1_8, helper_vfp_ah_maxd)
+
+DO_ZPZZ_PAIR_FP(sve2_ah_fminp_zpzz_h, float16, H1_2, helper_vfp_ah_minh)
+DO_ZPZZ_PAIR_FP(sve2_ah_fminp_zpzz_s, float32, H1_4, helper_vfp_ah_mins)
+DO_ZPZZ_PAIR_FP(sve2_ah_fminp_zpzz_d, float64, H1_8, helper_vfp_ah_mind)
+
 #undef DO_ZPZZ_PAIR_FP
 
 /* Three-operand expander, controlled by a predicate, in which the
diff --git a/target/arm/tcg/translate-sve.c b/target/arm/tcg/translate-sve.c
index aa7d72a35e..bf9f0ae179 100644
--- a/target/arm/tcg/translate-sve.c
+++ b/target/arm/tcg/translate-sve.c
@@ -7667,8 +7667,8 @@ TRANS_FEAT_NONSTREAMING(HISTSEG, aa64_sve2, gen_gvec_ool_arg_zzz,
 DO_ZPZZ_FP(FADDP, aa64_sme_or_sve2, sve2_faddp_zpzz)
 DO_ZPZZ_FP(FMAXNMP, aa64_sme_or_sve2, sve2_fmaxnmp_zpzz)
 DO_ZPZZ_FP(FMINNMP, aa64_sme_or_sve2, sve2_fminnmp_zpzz)
-DO_ZPZZ_FP(FMAXP, aa64_sme_or_sve2, sve2_fmaxp_zpzz)
-DO_ZPZZ_FP(FMINP, aa64_sme_or_sve2, sve2_fminp_zpzz)
+DO_ZPZZ_AH_FP(FMAXP, aa64_sme_or_sve2, sve2_fmaxp_zpzz, sve2_ah_fmaxp_zpzz)
+DO_ZPZZ_AH_FP(FMINP, aa64_sme_or_sve2, sve2_fminp_zpzz, sve2_ah_fminp_zpzz)
 
 static bool do_fmmla(DisasContext *s, arg_rrrr_esz *a,
                      gen_helper_gvec_4_ptr *fn)
-- 
2.43.0

Re: [PATCH 1/3] target/arm: SVE2 FMAXP, FMINP must honour AH=1

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> The behaviour of floating-point maximum and minimum insns has
> some odd special cases when FPCR.AH=1. We get this right in most
> places (for instance, the ASIMD FMAXP, FMINP) but forgot about
> it for the SVE2 versions of FMAXP and FMINP.
>
> Cc: qemu-stable@nongnu.org
> Fixes: 384433e70983 ("target/arm: Implement FPCR.AH semantics for FMINP and FMAXP")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH 2/3] target/arm: Use FPST_A64_F16 for SVE FCVTLT_hs

[Peter Maydell]

The SVE FCVTLT_hs operation produces a halfprec result, so we should
use the FPST_A64_F16 fpstatus for it.  Getting this wrong means we
will incorrectly set FPCR.IDC for input-denormals when FPCR.AH=1.

We missed this instruction when we updated the halfproc-to-other
conversion insns to use FPST_A64_F16 in commit e07b48995aaa
as part of implementing FEAT_AHP.

Cc: qemu-stable@nongnu.org
Fixes: e07b48995aaa ("target/arm: Use FPST_A64_F16 for halfprec-to-other conversions")a
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/tcg/translate-sve.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/tcg/translate-sve.c b/target/arm/tcg/translate-sve.c
index bf9f0ae179..59b8c494a8 100644
--- a/target/arm/tcg/translate-sve.c
+++ b/target/arm/tcg/translate-sve.c
@@ -7827,7 +7827,7 @@ TRANS_FEAT(BFCVTNT, aa64_sme_sve_bf16, gen_gvec_fpst_arg_zpz,
            s->fpcr_ah ? FPST_AH : FPST_A64)
 
 TRANS_FEAT(FCVTLT_hs, aa64_sme_or_sve2, gen_gvec_fpst_arg_zpz,
-           gen_helper_sve2_fcvtlt_hs, a, 0, FPST_A64)
+           gen_helper_sve2_fcvtlt_hs, a, 0, FPST_A64_F16)
 TRANS_FEAT(FCVTLT_sd, aa64_sme_or_sve2, gen_gvec_fpst_arg_zpz,
            gen_helper_sve2_fcvtlt_sd, a, 0, FPST_A64)
 
-- 
2.43.0

Re: [PATCH 2/3] target/arm: Use FPST_A64_F16 for SVE FCVTLT_hs

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> The SVE FCVTLT_hs operation produces a halfprec result, so we should
> use the FPST_A64_F16 fpstatus for it.

Maybe I'm reading the Arm ARM wrong (I can't find FCVTLT_hs but I can
see FCVTLT in the SVE encodings). But these are all widening operations
so I can see halfprec to single, not the other way around.

> Getting this wrong means we
> will incorrectly set FPCR.IDC for input-denormals when FPCR.AH=1.
>
> We missed this instruction when we updated the halfproc-to-other
> conversion insns to use FPST_A64_F16 in commit e07b48995aaa
> as part of implementing FEAT_AHP.
>
> Cc: qemu-stable@nongnu.org
> Fixes: e07b48995aaa ("target/arm: Use FPST_A64_F16 for halfprec-to-other conversions")a
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/tcg/translate-sve.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/arm/tcg/translate-sve.c b/target/arm/tcg/translate-sve.c
> index bf9f0ae179..59b8c494a8 100644
> --- a/target/arm/tcg/translate-sve.c
> +++ b/target/arm/tcg/translate-sve.c
> @@ -7827,7 +7827,7 @@ TRANS_FEAT(BFCVTNT, aa64_sme_sve_bf16, gen_gvec_fpst_arg_zpz,
>             s->fpcr_ah ? FPST_AH : FPST_A64)
>  
>  TRANS_FEAT(FCVTLT_hs, aa64_sme_or_sve2, gen_gvec_fpst_arg_zpz,
> -           gen_helper_sve2_fcvtlt_hs, a, 0, FPST_A64)
> +           gen_helper_sve2_fcvtlt_hs, a, 0, FPST_A64_F16)
>  TRANS_FEAT(FCVTLT_sd, aa64_sme_or_sve2, gen_gvec_fpst_arg_zpz,
>             gen_helper_sve2_fcvtlt_sd, a, 0, FPST_A64)

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 2/3] target/arm: Use FPST_A64_F16 for SVE FCVTLT_hs

[Peter Maydell]

On Thu, 21 May 2026 at 15:55, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > The SVE FCVTLT_hs operation produces a halfprec result, so we should
> > use the FPST_A64_F16 fpstatus for it.
>
> Maybe I'm reading the Arm ARM wrong (I can't find FCVTLT_hs but I can
> see FCVTLT in the SVE encodings). But these are all widening operations
> so I can see halfprec to single, not the other way around.

Yes, I got the commit message wrong here; this is half to single,
and we need to use the A64_F16 fpstatus for conversions from
halfprec, not conversions to it. The e07b48995aaa commit message
does a better job of explaining this than I did this time around.

The _hs is how the QEMU decode file splits the half to single
and the single to double encodings, which are the same insn
mnememonic in the architecture.

Improved commit message:

===begin===
We should be using the F16-specific float_status for conversions from
half-precision, because halfprec inputs never set Input Denormal.
If we use the FPST_A64 fpstatus then we will incorrectly set
FPCR.IDC for input-denormals when FPCR.AH=1.

In commit e07b48995aaa we updated most of the halfprec-to-other
conversion insns to use FPST_A64_F16 as part of implementing FEAT_AHP.
However we missed the SVE FCVTLT instruction, which has a
halfprec-to-single encoding.

Correct the FPST we use for the hs variant of FCVTLT.

[cc, fixes, &c]
===endit===


thanks
-- PMM

Re: [PATCH 2/3] target/arm: Use FPST_A64_F16 for SVE FCVTLT_hs

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Thu, 21 May 2026 at 15:55, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>> > The SVE FCVTLT_hs operation produces a halfprec result, so we should
>> > use the FPST_A64_F16 fpstatus for it.
>>
>> Maybe I'm reading the Arm ARM wrong (I can't find FCVTLT_hs but I can
>> see FCVTLT in the SVE encodings). But these are all widening operations
>> so I can see halfprec to single, not the other way around.
>
> Yes, I got the commit message wrong here; this is half to single,
> and we need to use the A64_F16 fpstatus for conversions from
> halfprec, not conversions to it. The e07b48995aaa commit message
> does a better job of explaining this than I did this time around.
>
> The _hs is how the QEMU decode file splits the half to single
> and the single to double encodings, which are the same insn
> mnememonic in the architecture.
>
> Improved commit message:
>
> ===begin===
> We should be using the F16-specific float_status for conversions from
> half-precision, because halfprec inputs never set Input Denormal.
> If we use the FPST_A64 fpstatus then we will incorrectly set
> FPCR.IDC for input-denormals when FPCR.AH=1.
>
> In commit e07b48995aaa we updated most of the halfprec-to-other
> conversion insns to use FPST_A64_F16 as part of implementing FEAT_AHP.
> However we missed the SVE FCVTLT instruction, which has a
> halfprec-to-single encoding.
>
> Correct the FPST we use for the hs variant of FCVTLT.
>
> [cc, fixes, &c]
> ===endit===
>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

>
> thanks
> -- PMM

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 2/3] target/arm: Use FPST_A64_F16 for SVE FCVTLT_hs

[Richard Henderson]

On 5/21/26 05:29, Peter Maydell wrote:
> The SVE FCVTLT_hs operation produces a halfprec result, so we should

halfprec input.

> use the FPST_A64_F16 fpstatus for it.  Getting this wrong means we
> will incorrectly set FPCR.IDC for input-denormals when FPCR.AH=1.
> 
> We missed this instruction when we updated the halfproc-to-other
> conversion insns to use FPST_A64_F16 in commit e07b48995aaa
> as part of implementing FEAT_AHP.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: e07b48995aaa ("target/arm: Use FPST_A64_F16 for halfprec-to-other conversions")a
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/tcg/translate-sve.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/arm/tcg/translate-sve.c b/target/arm/tcg/translate-sve.c
> index bf9f0ae179..59b8c494a8 100644
> --- a/target/arm/tcg/translate-sve.c
> +++ b/target/arm/tcg/translate-sve.c
> @@ -7827,7 +7827,7 @@ TRANS_FEAT(BFCVTNT, aa64_sme_sve_bf16, gen_gvec_fpst_arg_zpz,
>              s->fpcr_ah ? FPST_AH : FPST_A64)
>   
>   TRANS_FEAT(FCVTLT_hs, aa64_sme_or_sve2, gen_gvec_fpst_arg_zpz,
> -           gen_helper_sve2_fcvtlt_hs, a, 0, FPST_A64)
> +           gen_helper_sve2_fcvtlt_hs, a, 0, FPST_A64_F16)
>   TRANS_FEAT(FCVTLT_sd, aa64_sme_or_sve2, gen_gvec_fpst_arg_zpz,
>              gen_helper_sve2_fcvtlt_sd, a, 0, FPST_A64)
>   

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 3/3] target/arm: Set correct fp flags for FLOGB when FPCR.AH = 1

[Peter Maydell]

Our implementation of the FLOGB insn does the operations entirely
in the helper function, without needing to use fpu functions.
This means it needs to handle all the fp status flags itself.
We aren't setting float_flag_input_denormal_used when we
use (i.e. do not flush to zero) an input denormal, which means
that FPCR.IDC isn't set when it should be for FPCR.AH=1.
We missed this when we added float_flag_input_denormal_used
and made the fpu/ code set it.

Add the missing float_raise().

Cc: qemu-stable@nongnu.org
Fixes: d38a57a3f ("target/arm: Enable FEAT_AFP for '-cpu max'")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/tcg/sve_helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/arm/tcg/sve_helper.c b/target/arm/tcg/sve_helper.c
index 179cbd74fb..d884ba474f 100644
--- a/target/arm/tcg/sve_helper.c
+++ b/target/arm/tcg/sve_helper.c
@@ -5036,6 +5036,7 @@ static int16_t do_float16_logb_as_int(float16 a, float_status *s)
         if (frac != 0) {
             if (!get_flush_inputs_to_zero(s)) {
                 /* denormal: bias - fractional_zeros */
+                float_raise(float_flag_input_denormal_used, s);
                 return -15 - clz32(frac);
             }
             /* flush to zero */
@@ -5064,6 +5065,7 @@ static int32_t do_float32_logb_as_int(float32 a, float_status *s)
         if (frac != 0) {
             if (!get_flush_inputs_to_zero(s)) {
                 /* denormal: bias - fractional_zeros */
+                float_raise(float_flag_input_denormal_used, s);
                 return -127 - clz32(frac);
             }
             /* flush to zero */
@@ -5092,6 +5094,7 @@ static int64_t do_float64_logb_as_int(float64 a, float_status *s)
         if (frac != 0) {
             if (!get_flush_inputs_to_zero(s)) {
                 /* denormal: bias - fractional_zeros */
+                float_raise(float_flag_input_denormal_used, s);
                 return -1023 - clz64(frac);
             }
             /* flush to zero */
-- 
2.43.0

Re: [PATCH 3/3] target/arm: Set correct fp flags for FLOGB when FPCR.AH = 1

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Our implementation of the FLOGB insn does the operations entirely
> in the helper function, without needing to use fpu functions.
> This means it needs to handle all the fp status flags itself.
> We aren't setting float_flag_input_denormal_used when we
> use (i.e. do not flush to zero) an input denormal, which means
> that FPCR.IDC isn't set when it should be for FPCR.AH=1.
> We missed this when we added float_flag_input_denormal_used
> and made the fpu/ code set it.
>
> Add the missing float_raise().
>
> Cc: qemu-stable@nongnu.org
> Fixes: d38a57a3f ("target/arm: Enable FEAT_AFP for '-cpu max'")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 3/3] target/arm: Set correct fp flags for FLOGB when FPCR.AH = 1

[Richard Henderson]

On 5/21/26 05:29, Peter Maydell wrote:
> Our implementation of the FLOGB insn does the operations entirely
> in the helper function, without needing to use fpu functions.
> This means it needs to handle all the fp status flags itself.
> We aren't setting float_flag_input_denormal_used when we
> use (i.e. do not flush to zero) an input denormal, which means
> that FPCR.IDC isn't set when it should be for FPCR.AH=1.
> We missed this when we added float_flag_input_denormal_used
> and made the fpu/ code set it.
> 
> Add the missing float_raise().
> 
> Cc:qemu-stable@nongnu.org
> Fixes: d38a57a3f ("target/arm: Enable FEAT_AFP for '-cpu max'")
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/tcg/sve_helper.c | 3 +++
>   1 file changed, 3 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~