Re: [Qemu-devel] [PATCH] fpu_helper.c: fix helper_fpscr_clrbit() function

[David Gibson <david@gibson.dropbear.id.au>]

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

On Sun, Jun 17, 2018 at 11:53:09AM -0400, John Arbuckle wrote:
> Fix the helper_fpscr_clrbit() function so it correctly
> sets the FEX and VX bits.

This needs a lot more information in the commit message:

  * What exactly was wrong with the previous setting of the FEX and VX
    bits?

  * Where can we find documentation which describes how they should be
    set correctly?
  
This is information we need to properly review the patch.

> 
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> ---
>  target/ppc/fpu_helper.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
> index d31a933cbb..7e697a11d0 100644
> --- a/target/ppc/fpu_helper.c
> +++ b/target/ppc/fpu_helper.c
> @@ -325,6 +325,63 @@ void helper_fpscr_clrbit(CPUPPCState *env, uint32_t bit)
>          case FPSCR_RN:
>              fpscr_set_rounding_mode(env);
>              break;
> +        case FPSCR_VXSNAN:
> +        case FPSCR_VXISI:
> +        case FPSCR_VXIDI:
> +        case FPSCR_VXZDZ:
> +        case FPSCR_VXIMZ:
> +        case FPSCR_VXVC:
> +        case FPSCR_VXSOFT:
> +        case FPSCR_VXSQRT:
> +        case FPSCR_VXCVI:
> +        {
> +            int vxsnan, vxisi, vxidi, vxzdz, vximz, vxvc, vxsoft, vxsqrt, vxcvi;
> +            vxsnan = (env->fpscr >> (31 - FPSCR_VXSNAN)) & 1;
> +            vxisi = (env->fpscr >> (31 - FPSCR_VXISI)) & 1;
> +            vxidi = (env->fpscr >> (31 - FPSCR_VXIDI)) & 1;
> +            vxzdz = (env->fpscr >> (31 - FPSCR_VXZDZ)) & 1;
> +            vximz = (env->fpscr >> (31 - FPSCR_VXIMZ)) & 1;
> +            vxvc = (env->fpscr >> (31 - FPSCR_VXVC)) & 1;
> +            vxsoft = (env->fpscr >> (31 - FPSCR_VXSOFT)) & 1;
> +            vxsqrt = (env->fpscr >> (31 - FPSCR_VXSQRT)) & 1;
> +            vxcvi = (env->fpscr >> (31 - FPSCR_VXCVI)) & 1;
> +            if (~(vxsnan & vxisi & vxidi & vxzdz & vximz & vxvc & vxsoft &
> +                  vxsqrt & vxcvi)) {
> +                /* Set VX bit to zero */
> +                env->fpscr = env->fpscr & ~(1 << FPSCR_VX);
> +            }
> +        }
> +            break;
> +        case FPSCR_VX:
> +        case FPSCR_OX:
> +        case FPSCR_UX:
> +        case FPSCR_ZX:
> +        case FPSCR_XX:
> +        case FPSCR_VE:
> +        case FPSCR_OE:
> +        case FPSCR_UE:
> +        case FPSCR_ZE:
> +        case FPSCR_XE:
> +        {
> +            int vx, ox, ux, zx, xx, ve, oe, ue, ze, xe;
> +            vx = (env->fpscr >> (31 - FPSCR_VX)) & 1;
> +            ox = (env->fpscr >> (31 - FPSCR_OX)) & 1;
> +            ux = (env->fpscr >> (31 - FPSCR_UX)) & 1;
> +            zx = (env->fpscr >> (31 - FPSCR_ZX)) & 1;
> +            xx = (env->fpscr >> (31 - FPSCR_XX)) & 1;
> +            ve = (env->fpscr >> (31 - FPSCR_VE)) & 1;
> +            oe = (env->fpscr >> (31 - FPSCR_OE)) & 1;
> +            ue = (env->fpscr >> (31 - FPSCR_UE)) & 1;
> +            ze = (env->fpscr >> (31 - FPSCR_ZE)) & 1;
> +            xe = (env->fpscr >> (31 - FPSCR_XE)) & 1;
> +            bool fex;
> +            fex = (vx & ve) | (ox & oe) | (ux & ue) | (zx & ze) | (xx & xe);
> +            unsigned int mask;
> +            mask = (1 << FPSCR_FEX);
> +            /* Set the FEX bit */
> +            env->fpscr = (env->fpscr & ~mask) | (-fex & mask);
> +        }
> +            break;
>          default:
>              break;
>          }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]