public inbox for qemu-devel@nongnu.org
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: Lin Li-Hang <lihang.lin@gmail.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [PATCH] target/arm/tcg/vfp_helper: Fix incorrect bit field deposition in rsqrte_f64
Date: Mon, 12 Jan 2026 14:36:35 +0000	[thread overview]
Message-ID: <CAFEAcA-NSqMH_+L+jOCD-QsafFq78L5ogRmGRqVZnXdXja2WTA@mail.gmail.com> (raw)
In-Reply-To: <CAPR+-WF6uSsm6nBY7jmmxKEMSHR3t+d4KG5KAtsDNJPxen-r9A@mail.gmail.com>

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


  parent reply	other threads:[~2026-01-12 14:37 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-26  6:42 [PATCH] target/arm/tcg/vfp_helper: Fix incorrect bit field deposition in rsqrte_f64 Li-Hang Lin
2025-12-26 15:19 ` Philippe Mathieu-Daudé
2025-12-28  8:13   ` Michael Tokarev
2025-12-28 17:15 ` Peter Maydell
2025-12-29  4:19   ` Lin Li-Hang
2026-01-08  1:11     ` Lin Li-Hang
2026-01-12 14:36     ` Peter Maydell [this message]
2026-01-13  3:34       ` Lin Li-Hang
2026-03-22  6:20 ` Michael Tokarev
2026-03-22 14:37   ` Lin Li-Hang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFEAcA-NSqMH_+L+jOCD-QsafFq78L5ogRmGRqVZnXdXja2WTA@mail.gmail.com \
    --to=peter.maydell@linaro.org \
    --cc=lihang.lin@gmail.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox