On Mon, 29 Dec 2025 at 04:19, Lin Li-Hang <[email protected]> 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

Reply via email to