RE: [Ping] [PATCH, 7/10] aarch64: add function to output ccmp insn

2014-10-27 Thread Zhenqiang Chen 


 -Original Message-
 From: Richard Henderson [mailto:r...@redhat.com]
 Sent: Sunday, October 12, 2014 12:52 PM
 To: Zhenqiang Chen; gcc-patches@gcc.gnu.org
 Subject: Re: [Ping] [PATCH, 7/10] aarch64: add function to output ccmp
insn
 
 On 10/11/2014 09:11 AM, Richard Henderson wrote:
  On 09/22/2014 11:45 PM, Zhenqiang Chen wrote:
  +static unsigned int
  +aarch64_code_to_nzcv (enum rtx_code code, bool inverse) {
  +  switch (code)
  +{
  +case NE: /* NE, Z == 0.  */
  +  return inverse ? AARCH64_CC_Z : 0;
  +case EQ: /* EQ, Z == 1.  */
  +  return inverse ? 0 : AARCH64_CC_Z;
  +case LE: /* LE, !(Z == 0  N == V).  */
  +  return inverse ? AARCH64_CC_N | AARCH64_CC_V : AARCH64_CC_Z;
  +case GT: /* GT, Z == 0  N == V.  */
  +  return inverse ? AARCH64_CC_Z : AARCH64_CC_N | AARCH64_CC_V;
  +case LT: /* LT, N != V.  */
  +  return inverse ? AARCH64_CC_N | AARCH64_CC_V : AARCH64_CC_N;
  +case GE: /* GE, N == V.  */
  +  return inverse ? AARCH64_CC_N : AARCH64_CC_N | AARCH64_CC_V;
  +case LEU: /* LS, !(C == 1  Z == 0).  */
  +  return inverse ? AARCH64_CC_C: AARCH64_CC_Z;
  +case GTU: /* HI, C ==1  Z == 0.  */
  +  return inverse ? AARCH64_CC_Z : AARCH64_CC_C;
  +case LTU: /* CC, C == 0.  */
  +  return inverse ? AARCH64_CC_C : 0;
  +case GEU: /* CS, C == 1.  */
  +  return inverse ? 0 : AARCH64_CC_C;
  +default:
  +  gcc_unreachable ();
  +  return 0;
  +}
  +}
  +
 
  I'm not overly fond of this, since code doesn't map 1-1.  It needs
  the context of a mode to provide a unique mapping.
 
  I think it would be better to rearrange the existing aarch64_cond_code
  enum such that AARCH64_NE et al are meaningful wrt NZCV.  Then you can
  use
  aarch64_get_condition_code_1 to get this mapping.

 Slight mistake in the advice here.  I think you should use
 aarch64_get_conditional_code_1 to get an aarch64_cond_code, and use that
 to index an array to get the nzcv bits.

Thanks for the comments. Patch is updated.
 
 Further, does it actually make sense to store both nzcv and its inverse,
or
 does it work to use nzcv and ~nzcv?
 
Flag GE checks N == V and LT checks N != V. We can no distinguish them with
nzcv and ~nzcv

Since aarch64_output_ccmp is removed as your comments, the patch is combined
with [PATCH, 8/10] aarch64: ccmp insn patterns. Please find the updated
patch in later email.

Thanks!
-Zhenqiang

Re: [Ping] [PATCH, 7/10] aarch64: add function to output ccmp insn

2014-10-11 Thread Richard Henderson 
On 09/22/2014 11:45 PM, Zhenqiang Chen wrote:
 +static unsigned int
 +aarch64_code_to_nzcv (enum rtx_code code, bool inverse) {
 +  switch (code)
 +{
 +case NE: /* NE, Z == 0.  */
 +  return inverse ? AARCH64_CC_Z : 0;
 +case EQ: /* EQ, Z == 1.  */
 +  return inverse ? 0 : AARCH64_CC_Z;
 +case LE: /* LE, !(Z == 0  N == V).  */
 +  return inverse ? AARCH64_CC_N | AARCH64_CC_V : AARCH64_CC_Z;
 +case GT: /* GT, Z == 0  N == V.  */
 +  return inverse ? AARCH64_CC_Z : AARCH64_CC_N | AARCH64_CC_V;
 +case LT: /* LT, N != V.  */
 +  return inverse ? AARCH64_CC_N | AARCH64_CC_V : AARCH64_CC_N;
 +case GE: /* GE, N == V.  */
 +  return inverse ? AARCH64_CC_N : AARCH64_CC_N | AARCH64_CC_V;
 +case LEU: /* LS, !(C == 1  Z == 0).  */
 +  return inverse ? AARCH64_CC_C: AARCH64_CC_Z;
 +case GTU: /* HI, C ==1  Z == 0.  */
 +  return inverse ? AARCH64_CC_Z : AARCH64_CC_C;
 +case LTU: /* CC, C == 0.  */
 +  return inverse ? AARCH64_CC_C : 0;
 +case GEU: /* CS, C == 1.  */
 +  return inverse ? 0 : AARCH64_CC_C;
 +default:
 +  gcc_unreachable ();
 +  return 0;
 +}
 +}
 +

I'm not overly fond of this, since code doesn't map 1-1.  It needs the
context of a mode to provide a unique mapping.

I think it would be better to rearrange the existing aarch64_cond_code enum
such that AARCH64_NE et al are meaningful wrt NZCV.  Then you can use
aarch64_get_condition_code_1 to get this mapping.

 +static unsigned
 +aarch64_mode_to_condition_code (enum machine_mode mode, bool
 inverse) {
 +  switch (mode)
 +{
 +case CC_DNEmode:
 +  return inverse ? aarch64_get_condition_code_1 (CCmode, EQ)
 +  : aarch64_get_condition_code_1 (CCmode, NE);

This function is just silly.  Modulo the unsigned result, which is wrong after
the rebase, the whole thing reduces to

  return aarch64_get_condition_code_1 (mode, inverse ? EQ : NE);

I'm really not sure what you're after here.

 +const char *
 +aarch64_output_ccmp (rtx *operands, bool is_and, int which_alternative)

Is this really used more than once?  I'm not fond of the use of
which_alternative without the context of a pattern.  I think this could simply
be inlined.


r~

Re: [Ping] [PATCH, 7/10] aarch64: add function to output ccmp insn

2014-10-11 Thread Richard Henderson 
On 10/11/2014 09:11 AM, Richard Henderson wrote:
 On 09/22/2014 11:45 PM, Zhenqiang Chen wrote:
 +static unsigned int
 +aarch64_code_to_nzcv (enum rtx_code code, bool inverse) {
 +  switch (code)
 +{
 +case NE: /* NE, Z == 0.  */
 +  return inverse ? AARCH64_CC_Z : 0;
 +case EQ: /* EQ, Z == 1.  */
 +  return inverse ? 0 : AARCH64_CC_Z;
 +case LE: /* LE, !(Z == 0  N == V).  */
 +  return inverse ? AARCH64_CC_N | AARCH64_CC_V : AARCH64_CC_Z;
 +case GT: /* GT, Z == 0  N == V.  */
 +  return inverse ? AARCH64_CC_Z : AARCH64_CC_N | AARCH64_CC_V;
 +case LT: /* LT, N != V.  */
 +  return inverse ? AARCH64_CC_N | AARCH64_CC_V : AARCH64_CC_N;
 +case GE: /* GE, N == V.  */
 +  return inverse ? AARCH64_CC_N : AARCH64_CC_N | AARCH64_CC_V;
 +case LEU: /* LS, !(C == 1  Z == 0).  */
 +  return inverse ? AARCH64_CC_C: AARCH64_CC_Z;
 +case GTU: /* HI, C ==1  Z == 0.  */
 +  return inverse ? AARCH64_CC_Z : AARCH64_CC_C;
 +case LTU: /* CC, C == 0.  */
 +  return inverse ? AARCH64_CC_C : 0;
 +case GEU: /* CS, C == 1.  */
 +  return inverse ? 0 : AARCH64_CC_C;
 +default:
 +  gcc_unreachable ();
 +  return 0;
 +}
 +}
 +
 
 I'm not overly fond of this, since code doesn't map 1-1.  It needs the
 context of a mode to provide a unique mapping.
 
 I think it would be better to rearrange the existing aarch64_cond_code enum
 such that AARCH64_NE et al are meaningful wrt NZCV.  Then you can use
 aarch64_get_condition_code_1 to get this mapping.

Slight mistake in the advice here.  I think you should use
aarch64_get_conditional_code_1 to get an aarch64_cond_code, and use that to
index an array to get the nzcv bits.

Further, does it actually make sense to store both nzcv and its inverse, or
does it work to use nzcv and ~nzcv?


r~

[Ping] [PATCH, 7/10] aarch64: add function to output ccmp insn

2014-09-23 Thread Zhenqiang Chen 
Ping?

Patch is attached for easy to apply.

Thanks!
-Zhenqiang

 -Original Message-
 From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
 ow...@gcc.gnu.org] On Behalf Of Zhenqiang Chen
 Sent: Monday, June 23, 2014 3:00 PM
 To: gcc-patches@gcc.gnu.org
 Subject: [PATCH, 7/10] aarch64: add function to output ccmp insn
 
 Hi,
 
 The patch adds three help functions to output ccmp instructions.
 
 OK for trunk?
 
 Thanks!
 -Zhenqiang
 
 ChangeLog:
 2014-06-23  Zhenqiang Chen  zhenqiang.c...@linaro.org
 
 * config/aarch64/aarch64-protos.h (aarch64_output_ccmp): New
 prototype.
 * config/aarch64/aarch64.c (aarch64_code_to_nzcv): New function.
 (aarch64_mode_to_condition_code): New function.
 (aarch64_output_ccmp): New function.
 
 diff --git a/gcc/config/aarch64/aarch64-protos.h
 b/gcc/config/aarch64/aarch64-protos.h
 index 997ff50..ff1a0f4 100644
 --- a/gcc/config/aarch64/aarch64-protos.h
 +++ b/gcc/config/aarch64/aarch64-protos.h
 @@ -247,6 +247,7 @@ void aarch64_print_operand (FILE *, rtx, char);  void
 aarch64_print_operand_address (FILE *, rtx);
 
  bool aarch64_uimm5 (HOST_WIDE_INT);
 +const char* aarch64_output_ccmp (rtx *, bool, int);
 
  /* Initialize builtins for SIMD intrinsics.  */  void
init_aarch64_simd_builtins
 (void); diff --git a/gcc/config/aarch64/aarch64.c
 b/gcc/config/aarch64/aarch64.c index e5ede6e..5fe4826 100644
 --- a/gcc/config/aarch64/aarch64.c
 +++ b/gcc/config/aarch64/aarch64.c
 @@ -9602,6 +9602,137 @@ aarch64_uimm5 (HOST_WIDE_INT val)
return (val  (HOST_WIDE_INT) 0x1f) == val;  }
 
 +/* N Z C V.  */
 +#define AARCH64_CC_V 1
 +#define AARCH64_CC_C (1  1)
 +#define AARCH64_CC_Z (1  2)
 +#define AARCH64_CC_N (1  3)
 +
 +static unsigned int
 +aarch64_code_to_nzcv (enum rtx_code code, bool inverse) {
 +  switch (code)
 +{
 +case NE: /* NE, Z == 0.  */
 +  return inverse ? AARCH64_CC_Z : 0;
 +case EQ: /* EQ, Z == 1.  */
 +  return inverse ? 0 : AARCH64_CC_Z;
 +case LE: /* LE, !(Z == 0  N == V).  */
 +  return inverse ? AARCH64_CC_N | AARCH64_CC_V : AARCH64_CC_Z;
 +case GT: /* GT, Z == 0  N == V.  */
 +  return inverse ? AARCH64_CC_Z : AARCH64_CC_N | AARCH64_CC_V;
 +case LT: /* LT, N != V.  */
 +  return inverse ? AARCH64_CC_N | AARCH64_CC_V : AARCH64_CC_N;
 +case GE: /* GE, N == V.  */
 +  return inverse ? AARCH64_CC_N : AARCH64_CC_N | AARCH64_CC_V;
 +case LEU: /* LS, !(C == 1  Z == 0).  */
 +  return inverse ? AARCH64_CC_C: AARCH64_CC_Z;
 +case GTU: /* HI, C ==1  Z == 0.  */
 +  return inverse ? AARCH64_CC_Z : AARCH64_CC_C;
 +case LTU: /* CC, C == 0.  */
 +  return inverse ? AARCH64_CC_C : 0;
 +case GEU: /* CS, C == 1.  */
 +  return inverse ? 0 : AARCH64_CC_C;
 +default:
 +  gcc_unreachable ();
 +  return 0;
 +}
 +}
 +
 +static unsigned
 +aarch64_mode_to_condition_code (enum machine_mode mode, bool
 inverse) {
 +  switch (mode)
 +{
 +case CC_DNEmode:
 +  return inverse ? aarch64_get_condition_code_1 (CCmode, EQ)
 +  : aarch64_get_condition_code_1 (CCmode, NE);
 +case CC_DEQmode:
 +  return inverse ? aarch64_get_condition_code_1 (CCmode, NE)
 +  : aarch64_get_condition_code_1 (CCmode, EQ);
 +case CC_DLEmode:
 +  return inverse ? aarch64_get_condition_code_1 (CCmode, GT)
 +  : aarch64_get_condition_code_1 (CCmode, LE);
 +case CC_DGTmode:
 +  return inverse ? aarch64_get_condition_code_1 (CCmode, LE)
 +  : aarch64_get_condition_code_1 (CCmode, GT);
 +case CC_DLTmode:
 +  return inverse ? aarch64_get_condition_code_1 (CCmode, GE)
 +  : aarch64_get_condition_code_1 (CCmode, LT);
 +case CC_DGEmode:
 +  return inverse ? aarch64_get_condition_code_1 (CCmode, LT)
 +  : aarch64_get_condition_code_1 (CCmode, GE);
 +case CC_DLEUmode:
 +  return inverse ? aarch64_get_condition_code_1 (CCmode, GTU)
 +  : aarch64_get_condition_code_1 (CCmode, LEU);
 +case CC_DGTUmode:
 +  return inverse ? aarch64_get_condition_code_1 (CCmode, LEU)
 +  : aarch64_get_condition_code_1 (CCmode, GTU);
 +case CC_DLTUmode:
 +  return inverse ? aarch64_get_condition_code_1 (CCmode, GEU)
 +  : aarch64_get_condition_code_1 (CCmode, LTU);
 +case CC_DGEUmode:
 +  return inverse ? aarch64_get_condition_code_1 (CCmode, LTU)
 +  : aarch64_get_condition_code_1 (CCmode, GEU);
 +default:
 +  gcc_unreachable ();
 +}
 +}
 +
 +const char *
 +aarch64_output_ccmp (rtx *operands, bool is_and, int which_alternative)
 +{
 +  char buf[32];
 +  rtx cc = operands[0];
 +  enum rtx_code code = GET_CODE (operands[5]);
 +  unsigned char nzcv = aarch64_code_to_nzcv (code, is_and);
 +  enum machine_mode mode = GET_MODE (cc);
 +  unsigned int cond_code = aarch64_mode_to_condition_code (mode,
 +!is_and);
 +
 +