All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
Cc: qemu-ppc@nongnu.org, rth@twiddle.net, qemu-devel@nongnu.org,
	bharata@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [PATCH 2/9] target-ppc: Fix xscmpodp and xscmpudp instructions
Date: Wed, 23 Nov 2016 15:01:18 +1100	[thread overview]
Message-ID: <20161123040118.GK28479@umbus.fritz.box> (raw)
In-Reply-To: <1479815165-31059-3-git-send-email-nikunj@linux.vnet.ibm.com>

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

On Tue, Nov 22, 2016 at 05:15:58PM +0530, Nikunj A Dadhania wrote:
> From: Bharata B Rao <bharata@linux.vnet.ibm.com>
> 
> - xscmpodp & xscmpudp are missing flags reset.
> - In xscmpodp, VXCC should be set only if VE is 0 for signalling NaN case
>   and VXCC should be set by explicitly checking for quiet NaN case.
> - Comparison is being done only if the operands are not NaNs. However as
>   per ISA, it should be done even when operands are NaNs.

For my interest, can you explain the difference between ordered and
unordered comparisons?  I looked at the ISA and mostly just became
confused.

> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> Signed-off-by: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
> ---
>  target-ppc/fpu_helper.c | 41 +++++++++++++++++++++++++----------------
>  1 file changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
> index d3741b4..3027003 100644
> --- a/target-ppc/fpu_helper.c
> +++ b/target-ppc/fpu_helper.c
> @@ -2410,29 +2410,38 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)                      \
>  {                                                                        \
>      ppc_vsr_t xa, xb;                                                    \
>      uint32_t cc = 0;                                                     \
> +    bool vxsnan_flag = false, vxvc_flag = false;                         \
>                                                                           \
> +    helper_reset_fpstatus(env);                                          \
>      getVSR(xA(opcode), &xa, env);                                        \
>      getVSR(xB(opcode), &xb, env);                                        \
>                                                                           \
> -    if (unlikely(float64_is_any_nan(xa.VsrD(0)) ||                       \
> -                 float64_is_any_nan(xb.VsrD(0)))) {                      \
> -        if (float64_is_signaling_nan(xa.VsrD(0), &env->fp_status) ||     \
> -            float64_is_signaling_nan(xb.VsrD(0), &env->fp_status)) {     \
> -            float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 0);       \
> -        }                                                                \
> -        if (ordered) {                                                   \
> -            float_invalid_op_excp(env, POWERPC_EXCP_FP_VXVC, 0);         \
> +    if (float64_is_signaling_nan(xa.VsrD(0), &env->fp_status) ||         \
> +        float64_is_signaling_nan(xb.VsrD(0), &env->fp_status)) {         \
> +        vxsnan_flag = true;                                              \
> +        cc = 1;                                                          \
> +        if (fpscr_ve == 0 && ordered) {                                  \
> +            vxvc_flag = true;                                            \
>          }                                                                \
> +    } else if ((float64_is_quiet_nan(xa.VsrD(0), &env->fp_status) ||     \
> +                float64_is_quiet_nan(xb.VsrD(0), &env->fp_status))       \
> +               && ordered) {                                             \
>          cc = 1;                                                          \

Since you're basically rewriting this, could you please change it to
use symbolic constants for the CC bits, which will make it easier to
follow.

> +        vxvc_flag = true;                                                \
> +    }                                                                    \
> +    if (vxsnan_flag) {                                                   \
> +        float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 0);           \
> +    }                                                                    \
> +    if (vxvc_flag) {                                                     \
> +        float_invalid_op_excp(env, POWERPC_EXCP_FP_VXVC, 0);             \
> +    }                                                                    \
> +                                                                         \
> +    if (float64_lt(xa.VsrD(0), xb.VsrD(0), &env->fp_status)) {           \
> +        cc |= 8;                                                         \
> +    } else if (!float64_le(xa.VsrD(0), xb.VsrD(0), &env->fp_status)) {   \
> +        cc |= 4;                                                         \
>      } else {                                                             \
> -        if (float64_lt(xa.VsrD(0), xb.VsrD(0), &env->fp_status)) {       \
> -            cc = 8;                                                      \
> -        } else if (!float64_le(xa.VsrD(0), xb.VsrD(0),                   \
> -                               &env->fp_status)) { \
> -            cc = 4;                                                      \
> -        } else {                                                         \
> -            cc = 2;                                                      \
> -        }                                                                \
> +        cc |= 2;                                                         \
>      }                                                                    \
>                                                                           \
>      env->fpscr &= ~(0x0F << FPSCR_FPRF);                                 \

-- 
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: 819 bytes --]

  reply	other threads:[~2016-11-23  4:12 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-22 11:45 [Qemu-devel] [PATCH ppc-for-2.9 0/9] POWER9 TCG enablements - part8 Nikunj A Dadhania
2016-11-22 11:45 ` [Qemu-devel] [PATCH 1/9] target-ppc: Consolidate instruction decode helpers Nikunj A Dadhania
2016-11-23  3:56   ` David Gibson
2016-11-22 11:45 ` [Qemu-devel] [PATCH 2/9] target-ppc: Fix xscmpodp and xscmpudp instructions Nikunj A Dadhania
2016-11-23  4:01   ` David Gibson [this message]
2016-11-23  5:40     ` Bharata B Rao
2016-11-24  1:29       ` David Gibson
2016-11-22 11:45 ` [Qemu-devel] [PATCH 3/9] target-ppc: Add xscmpexp[dp, qp] instructions Nikunj A Dadhania
2016-11-23  4:06   ` David Gibson
2016-11-22 11:46 ` [Qemu-devel] [PATCH 4/9] target-ppc: Add xscmpoqp and xscmpuqp instructions Nikunj A Dadhania
2016-11-23  4:06   ` David Gibson
2016-11-22 11:46 ` [Qemu-devel] [PATCH 5/9] target-ppc: implement lxsd and lxssp instructions Nikunj A Dadhania
2016-11-23  4:06   ` David Gibson
2016-11-22 11:46 ` [Qemu-devel] [PATCH 6/9] target-ppc: implement stxsd and stxssp Nikunj A Dadhania
2016-11-22 15:19   ` Nikunj A Dadhania
2016-11-22 11:46 ` [Qemu-devel] [PATCH 7/9] target-ppc: implement lxv/lxvx and stxv/stxvx Nikunj A Dadhania
2016-11-22 11:46 ` [Qemu-devel] [PATCH 8/9] target-ppc: add vextu[bhw]lx instructions Nikunj A Dadhania
2016-11-23  4:11   ` David Gibson
2016-11-23  4:48     ` Nikunj A Dadhania
2016-11-22 11:46 ` [Qemu-devel] [PATCH 9/9] target-ppc: add vextu[bhw]rx instructions Nikunj A Dadhania

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=20161123040118.GK28479@umbus.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=bharata@linux.vnet.ibm.com \
    --cc=nikunj@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=rth@twiddle.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.