[PATCH] target/arm/tcg/vfp_helper: Fix incorrect bit field deposition in rsqrte_f64

[PATCH] target/arm/tcg/vfp_helper: Fix incorrect bit field deposition in rsqrte_f64

[Li-Hang Lin]

Fix an error in rsqrte_f64() where the sign bit was being
placed incorrectly. Specifically, ensure f64_sign is deposited
into bit 63.

Additionally, update the comments regarding the exponent (exp) bit
width to reflect the correct double-precision specifications.

Signed-off-by: Li-Hang Lin <lihang.lin@gmail.com>
---
 target/arm/tcg/vfp_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/tcg/vfp_helper.c b/target/arm/tcg/vfp_helper.c
index e156e3774a..60188b2c7e 100644
--- a/target/arm/tcg/vfp_helper.c
+++ b/target/arm/tcg/vfp_helper.c
@@ -1078,8 +1078,8 @@ float64 HELPER(rsqrte_f64)(float64 input, float_status *s)
 
     f64_frac = recip_sqrt_estimate(&f64_exp, 3068, f64_frac, false);
 
-    /* result = sign : result_exp<4:0> : estimate<7:0> : Zeros(44) */
-    val = deposit64(0, 61, 1, f64_sign);
+    /* result = sign : result_exp<10:0> : estimate<7:0> : Zeros(44) */
+    val = deposit64(0, 63, 1, f64_sign);
     val = deposit64(val, 52, 11, f64_exp);
     val = deposit64(val, 44, 8, extract64(f64_frac, 52 - 8, 8));
     return make_float64(val);
-- 
2.52.0.351.gbe84eed79e-goog

Re: [PATCH] target/arm/tcg/vfp_helper: Fix incorrect bit field deposition in rsqrte_f64

[Philippe Mathieu-Daudé]

On 26/12/25 07:42, Li-Hang Lin wrote:
> Fix an error in rsqrte_f64() where the sign bit was being
> placed incorrectly. Specifically, ensure f64_sign is deposited
> into bit 63.
> 
> Additionally, update the comments regarding the exponent (exp) bit
> width to reflect the correct double-precision specifications.
> 

Fixes: d719cbc7641 ("arm/helper.c: re-factor rsqrte and add rsqrte_f16")

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> Signed-off-by: Li-Hang Lin <lihang.lin@gmail.com>
> ---
>   target/arm/tcg/vfp_helper.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/tcg/vfp_helper.c b/target/arm/tcg/vfp_helper.c
> index e156e3774a..60188b2c7e 100644
> --- a/target/arm/tcg/vfp_helper.c
> +++ b/target/arm/tcg/vfp_helper.c
> @@ -1078,8 +1078,8 @@ float64 HELPER(rsqrte_f64)(float64 input, float_status *s)
>   
>       f64_frac = recip_sqrt_estimate(&f64_exp, 3068, f64_frac, false);
>   
> -    /* result = sign : result_exp<4:0> : estimate<7:0> : Zeros(44) */
> -    val = deposit64(0, 61, 1, f64_sign);
> +    /* result = sign : result_exp<10:0> : estimate<7:0> : Zeros(44) */
> +    val = deposit64(0, 63, 1, f64_sign);
>       val = deposit64(val, 52, 11, f64_exp);
>       val = deposit64(val, 44, 8, extract64(f64_frac, 52 - 8, 8));
>       return make_float64(val);

Re: [PATCH] target/arm/tcg/vfp_helper: Fix incorrect bit field deposition in rsqrte_f64

[Michael Tokarev]

On 12/26/25 18:19, Philippe Mathieu-Daudé wrote:
> On 26/12/25 07:42, Li-Hang Lin wrote:
>> Fix an error in rsqrte_f64() where the sign bit was being
>> placed incorrectly. Specifically, ensure f64_sign is deposited
>> into bit 63.
>>
>> Additionally, update the comments regarding the exponent (exp) bit
>> width to reflect the correct double-precision specifications.
>>
> 
> Fixes: d719cbc7641 ("arm/helper.c: re-factor rsqrte and add rsqrte_f16")

That's.. 2.12, 2018 - not too far ago :)

Cc: qemu-stable@nongnu.org.

/mjt

Re: [PATCH] target/arm/tcg/vfp_helper: Fix incorrect bit field deposition in rsqrte_f64

[Peter Maydell]

On Fri, 26 Dec 2025 at 06:43, Li-Hang Lin <lihang.lin@gmail.com> wrote:
>
> Fix an error in rsqrte_f64() where the sign bit was being
> placed incorrectly. Specifically, ensure f64_sign is deposited
> into bit 63.
>
> Additionally, update the comments regarding the exponent (exp) bit
> width to reflect the correct double-precision specifications.

This seems like it would produce incorrect results -- do you
have an example of an instruction plus input data values that p
produces a different output value to the hardware?

thanks
-- PMM

Re: [PATCH] target/arm/tcg/vfp_helper: Fix incorrect bit field deposition in rsqrte_f64

[Lin Li-Hang]

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]

Hi Peter,

Thank you for your reply.

I initially identified this error while reviewing the code and was curious
why it hadn't caused any bugs.
After further testing, it appears the original code behaved correctly by
coincidence.

The ASL code in the ARM ARM for FRSQRTE states:

```
elsif sign == '1' then
         result = FPDefaultNaN(fpcr, N);
         if fpexc then FPProcessException(FPExc_InvalidOp, fpcr);
```

As it turns out, the sign bit must be zero by the time it reaches the final
deposition code, which explains why the incorrect bit placement did not
surface as a functional bug.


On Mon, Dec 29, 2025 at 1:15 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Fri, 26 Dec 2025 at 06:43, Li-Hang Lin <lihang.lin@gmail.com> wrote:
> >
> > Fix an error in rsqrte_f64() where the sign bit was being
> > placed incorrectly. Specifically, ensure f64_sign is deposited
> > into bit 63.
> >
> > Additionally, update the comments regarding the exponent (exp) bit
> > width to reflect the correct double-precision specifications.
>
> This seems like it would produce incorrect results -- do you
> have an example of an instruction plus input data values that p
> produces a different output value to the hardware?
>
> thanks
> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 1905 bytes --]

Re: [PATCH] target/arm/tcg/vfp_helper: Fix incorrect bit field deposition in rsqrte_f64

[Lin Li-Hang]

[-- Attachment #1: Type: text/plain, Size: 1488 bytes --]

Hi Peter,

Is my patch good for you?

Cheers


On Mon, Dec 29, 2025 at 12:19 PM Lin Li-Hang <lihang.lin@gmail.com> wrote:

> Hi Peter,
>
> Thank you for your reply.
>
> I initially identified this error while reviewing the code and was curious
> why it hadn't caused any bugs.
> After further testing, it appears the original code behaved correctly by
> coincidence.
>
> The ASL code in the ARM ARM for FRSQRTE states:
>
> ```
> elsif sign == '1' then
>          result = FPDefaultNaN(fpcr, N);
>          if fpexc then FPProcessException(FPExc_InvalidOp, fpcr);
> ```
>
> As it turns out, the sign bit must be zero by the time it reaches the
> final deposition code, which explains why the incorrect bit placement did
> not surface as a functional bug.
>
>
> On Mon, Dec 29, 2025 at 1:15 AM Peter Maydell <peter.maydell@linaro.org>
> wrote:
>
>> On Fri, 26 Dec 2025 at 06:43, Li-Hang Lin <lihang.lin@gmail.com> wrote:
>> >
>> > Fix an error in rsqrte_f64() where the sign bit was being
>> > placed incorrectly. Specifically, ensure f64_sign is deposited
>> > into bit 63.
>> >
>> > Additionally, update the comments regarding the exponent (exp) bit
>> > width to reflect the correct double-precision specifications.
>>
>> This seems like it would produce incorrect results -- do you
>> have an example of an instruction plus input data values that p
>> produces a different output value to the hardware?
>>
>> thanks
>> -- PMM
>>
>

[-- Attachment #2: Type: text/html, Size: 2492 bytes --]

Re: [PATCH] target/arm/tcg/vfp_helper: Fix incorrect bit field deposition in rsqrte_f64

[Peter Maydell]

On Mon, 29 Dec 2025 at 04:19, Lin Li-Hang <lihang.lin@gmail.com> wrote:
>
> Hi Peter,
>
> Thank you for your reply.
>
> I initially identified this error while reviewing the code and was curious why it hadn't caused any bugs.
> After further testing, it appears the original code behaved correctly by coincidence.
>
> The ASL code in the ARM ARM for FRSQRTE states:
>
> ```
> elsif sign == '1' then
>          result = FPDefaultNaN(fpcr, N);
>          if fpexc then FPProcessException(FPExc_InvalidOp, fpcr);
> ```
>
> As it turns out, the sign bit must be zero by the time it reaches the final deposition code, which explains why the incorrect bit placement did not surface as a functional bug.

Thanks for looking that up. I think that although this isn't a bug it's
definitely confusing code, so the best approach will be to make our
code match how the current Arm ARM FPRSqrtEstimate() treats the output
sign bit, which is to say we know it's 0. In the pseudocode that
looks like:
  result = '0' : result_exp<N-54:0> : estimate<7:0>:Zeros(44);
and for QEMU it should look like updating the comment so that
instead of
/* result = sign : result_exp<4:0> : estimate<7:0> : Zeros(44) */

it says
/* result = 0 : result_exp<4:0> : estimate<7:0> : Zeros(44) */
and removing the unnecessary deposit64() call of f64_sign entirely.

We should do this for all of rsqrte_f64, do_rsqrte_f32 and rsqrte_f16.

thanks
-- PMM

Re: [PATCH] target/arm/tcg/vfp_helper: Fix incorrect bit field deposition in rsqrte_f64

[Lin Li-Hang]

[-- Attachment #1: Type: text/plain, Size: 1724 bytes --]

Hi Peter,

Thanks for your explanation.
Your statement and suggestion is better.

Cheers


On Mon, Jan 12, 2026 at 10:36 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Mon, 29 Dec 2025 at 04:19, Lin Li-Hang <lihang.lin@gmail.com> wrote:
> >
> > Hi Peter,
> >
> > Thank you for your reply.
> >
> > I initially identified this error while reviewing the code and was
> curious why it hadn't caused any bugs.
> > After further testing, it appears the original code behaved correctly by
> coincidence.
> >
> > The ASL code in the ARM ARM for FRSQRTE states:
> >
> > ```
> > elsif sign == '1' then
> >          result = FPDefaultNaN(fpcr, N);
> >          if fpexc then FPProcessException(FPExc_InvalidOp, fpcr);
> > ```
> >
> > As it turns out, the sign bit must be zero by the time it reaches the
> final deposition code, which explains why the incorrect bit placement did
> not surface as a functional bug.
>
> Thanks for looking that up. I think that although this isn't a bug it's
> definitely confusing code, so the best approach will be to make our
> code match how the current Arm ARM FPRSqrtEstimate() treats the output
> sign bit, which is to say we know it's 0. In the pseudocode that
> looks like:
>   result = '0' : result_exp<N-54:0> : estimate<7:0>:Zeros(44);
> and for QEMU it should look like updating the comment so that
> instead of
> /* result = sign : result_exp<4:0> : estimate<7:0> : Zeros(44) */
>
> it says
> /* result = 0 : result_exp<4:0> : estimate<7:0> : Zeros(44) */
> and removing the unnecessary deposit64() call of f64_sign entirely.
>
> We should do this for all of rsqrte_f64, do_rsqrte_f32 and rsqrte_f16.
>
> thanks
> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 2453 bytes --]

Re: [PATCH] target/arm/tcg/vfp_helper: Fix incorrect bit field deposition in rsqrte_f64

[Michael Tokarev]

On 26.12.2025 09:42, Li-Hang Lin wrote:
> Fix an error in rsqrte_f64() where the sign bit was being
> placed incorrectly. Specifically, ensure f64_sign is deposited
> into bit 63.
> 
> Additionally, update the comments regarding the exponent (exp) bit
> width to reflect the correct double-precision specifications.

Ping?  Or is this patch not needed anymore?

Thanks,

/mjt

>   target/arm/tcg/vfp_helper.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/tcg/vfp_helper.c b/target/arm/tcg/vfp_helper.c
> index e156e3774a..60188b2c7e 100644
> --- a/target/arm/tcg/vfp_helper.c
> +++ b/target/arm/tcg/vfp_helper.c
> @@ -1078,8 +1078,8 @@ float64 HELPER(rsqrte_f64)(float64 input, float_status *s)
>   
>       f64_frac = recip_sqrt_estimate(&f64_exp, 3068, f64_frac, false);
>   
> -    /* result = sign : result_exp<4:0> : estimate<7:0> : Zeros(44) */
> -    val = deposit64(0, 61, 1, f64_sign);
> +    /* result = sign : result_exp<10:0> : estimate<7:0> : Zeros(44) */
> +    val = deposit64(0, 63, 1, f64_sign);
>       val = deposit64(val, 52, 11, f64_exp);
>       val = deposit64(val, 44, 8, extract64(f64_frac, 52 - 8, 8));
>       return make_float64(val);

Re: [PATCH] target/arm/tcg/vfp_helper: Fix incorrect bit field deposition in rsqrte_f64

[Lin Li-Hang]

[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]

Hi mjt,

After discussing with Peter, I did further debugging.
The original code is harmless even with the wrong bit position placement.
My patch is not perfect and Peter suggested refactoring those snippets.

Cheers


On Sun, Mar 22, 2026 at 2:20 PM Michael Tokarev <mjt@tls.msk.ru> wrote:

> On 26.12.2025 09:42, Li-Hang Lin wrote:
> > Fix an error in rsqrte_f64() where the sign bit was being
> > placed incorrectly. Specifically, ensure f64_sign is deposited
> > into bit 63.
> >
> > Additionally, update the comments regarding the exponent (exp) bit
> > width to reflect the correct double-precision specifications.
>
> Ping?  Or is this patch not needed anymore?
>
> Thanks,
>
> /mjt
>
> >   target/arm/tcg/vfp_helper.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/arm/tcg/vfp_helper.c b/target/arm/tcg/vfp_helper.c
> > index e156e3774a..60188b2c7e 100644
> > --- a/target/arm/tcg/vfp_helper.c
> > +++ b/target/arm/tcg/vfp_helper.c
> > @@ -1078,8 +1078,8 @@ float64 HELPER(rsqrte_f64)(float64 input,
> float_status *s)
> >
> >       f64_frac = recip_sqrt_estimate(&f64_exp, 3068, f64_frac, false);
> >
> > -    /* result = sign : result_exp<4:0> : estimate<7:0> : Zeros(44) */
> > -    val = deposit64(0, 61, 1, f64_sign);
> > +    /* result = sign : result_exp<10:0> : estimate<7:0> : Zeros(44) */
> > +    val = deposit64(0, 63, 1, f64_sign);
> >       val = deposit64(val, 52, 11, f64_exp);
> >       val = deposit64(val, 44, 8, extract64(f64_frac, 52 - 8, 8));
> >       return make_float64(val);
>
>

[-- Attachment #2: Type: text/html, Size: 2282 bytes --]