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

Peter Maydell posted 3 patches 3 months ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>
[PATCH 2/3] target/arm: Use FPST_A64_F16 for SVE FCVTLT_hs
Posted by Peter Maydell 3 months ago
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
Posted by Richard Henderson 2 months, 4 weeks ago
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~
Re: [PATCH 2/3] target/arm: Use FPST_A64_F16 for SVE FCVTLT_hs
Posted by Alex Bennée 3 months ago
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
Posted by Peter Maydell 3 months ago
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
Posted by Alex Bennée 3 months ago
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