[Ping] [PATCH, 8/10] aarch64: ccmp insn patterns

[Ping] [PATCH, 8/10] aarch64: ccmp insn patterns

[Zhenqiang Chen]

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

Ping?

Patch is attached for easy to apply.

Move the cstorecc4 pattern from "[PATCH, 10/10] aarch64: Handle ccmp in
ifcvt to make it work with cmov" to this patch, since cmov optimization does
not depend on that patch.

Thanks!
-Zhenqiang

ChangeLog:
2014-09-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>

	* config/aarch64/aarch64.md (cbranchcc4, *ccmp_and, *ccmp_ior,
	cstorecc4): New.

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Zhenqiang Chen
> Sent: Monday, June 23, 2014 3:01 PM
> To: gcc-patches@gcc.gnu.org
> Subject: [PATCH, 8/10] aarch64: ccmp insn patterns
> 
> Hi,
> 
> The patch adds two insn patterns for ccmp instructions.
> 
> cbranchcc4 is introduced to generate optimized conditional branch without
> an additional compare against the result of ccmp.
> 
> OK for trunk?
> 
> Thanks!
> -Zhenqiang
> 
> ChangeLog:
> 2014-06-23  Zhenqiang Chen  <zhenqiang.chen@linaro.org>
> 
>         * config/aarch64/aarch64.md (cbranchcc4): New.
>         (*ccmp_and, *ccmp_ior): New.
> 
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index a4d8887..c25d940 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -230,6 +230,52 @@
>    "
>  )
> +(define_expand "cbranchcc4"
> +  [(set (pc) (if_then_else
> +             (match_operator 0 "aarch64_comparison_operator"
> +              [(match_operand 1 "cc_register" "")
> +               (const_int 0)])
> +             (label_ref (match_operand 3 "" ""))
> +             (pc)))]
> +  ""
> +  " ")
> +
> +(define_insn "*ccmp_and"
> +  [(set (match_operand 6 "ccmp_cc_register" "")
> +       (compare
> +        (and:SI
> +         (match_operator 4 "aarch64_comparison_operator"
> +          [(match_operand 0 "ccmp_cc_register" "")
> +           (match_operand 1 "aarch64_plus_operand" "")])
> +         (match_operator 5 "aarch64_comparison_operator"
> +          [(match_operand:GPI 2 "register_operand" "r,r,r")
> +           (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn")]))
> +        (const_int 0)))]
> +  ""
> +  {
> +    return aarch64_output_ccmp (operands, true, which_alternative);
> +  }
> +  [(set_attr "type" "alus_reg,alus_imm,alus_imm")]
> +)
> +
> +(define_insn "*ccmp_ior"
> +  [(set (match_operand 6 "ccmp_cc_register" "")
> +       (compare
> +        (ior:SI
> +         (match_operator 4 "aarch64_comparison_operator"
> +          [(match_operand 0 "ccmp_cc_register" "")
> +           (match_operand 1 "aarch64_plus_operand" "")])
> +         (match_operator 5 "aarch64_comparison_operator"
> +          [(match_operand:GPI 2 "register_operand" "r,r,r")
> +           (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn")]))
> +        (const_int 0)))]
> +  ""
> +  {
> +    return aarch64_output_ccmp (operands, false, which_alternative);
> +  }
> +  [(set_attr "type" "alus_reg,alus_imm,alus_imm")]
> +)
> +
>  (define_insn "*condjump"
>    [(set (pc) (if_then_else (match_operator 0
> "aarch64_comparison_operator"
>                             [(match_operand 1 "cc_register" "") (const_int
0)])
>                                                                    57,1

[-- Attachment #2: 8-patterns.patch --]
[-- Type: application/octet-stream, Size: 2322 bytes --]

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index c60038a..e213cbe 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -239,6 +239,52 @@
   "
 )
 
+(define_expand "cbranchcc4"
+  [(set (pc) (if_then_else
+	      (match_operator 0 "aarch64_comparison_operator"
+	       [(match_operand 1 "cc_register" "")
+	        (const_int 0)])
+	      (label_ref (match_operand 3 "" ""))
+	      (pc)))]
+  ""
+  " ")
+
+(define_insn "*ccmp_and"
+  [(set (match_operand 6 "ccmp_cc_register" "")
+	(compare
+	 (and:SI
+	  (match_operator 4 "aarch64_comparison_operator"
+	   [(match_operand 0 "ccmp_cc_register" "")
+	    (match_operand 1 "aarch64_plus_operand" "")])
+	  (match_operator 5 "aarch64_comparison_operator"
+	   [(match_operand:GPI 2 "register_operand" "r,r,r")
+	    (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn")]))
+	 (const_int 0)))]
+  ""
+  {
+    return aarch64_output_ccmp (operands, true, which_alternative);
+  }
+  [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
+)
+
+(define_insn "*ccmp_ior"
+  [(set (match_operand 6 "ccmp_cc_register" "")
+	(compare
+	 (ior:SI
+	  (match_operator 4 "aarch64_comparison_operator"
+	   [(match_operand 0 "ccmp_cc_register" "")
+	    (match_operand 1 "aarch64_plus_operand" "")])
+	  (match_operator 5 "aarch64_comparison_operator"
+	   [(match_operand:GPI 2 "register_operand" "r,r,r")
+	    (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn")]))
+	 (const_int 0)))]
+  ""
+  {
+    return aarch64_output_ccmp (operands, false, which_alternative);
+  }
+  [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
+)
+
 (define_insn "*condjump"
   [(set (pc) (if_then_else (match_operator 0 "aarch64_comparison_operator"
 			    [(match_operand 1 "cc_register" "") (const_int 0)])
@@ -2382,6 +2428,19 @@
   "
 )
 
+(define_expand "cstorecc4"
+  [(set (match_operand:SI 0 "register_operand")
+       (match_operator 1 "aarch64_comparison_operator"
+        [(match_operand 2 "ccmp_cc_register")
+         (match_operand 3 "aarch64_plus_operand")]))]
+  ""
+"{
+  operands[3] = const0_rtx;
+  emit_insn (gen_rtx_SET (SImode, operands[0], operands[1]));
+  DONE;
+}")
+
+
 (define_expand "cstore<mode>4"
   [(set (match_operand:SI 0 "register_operand" "")
 	(match_operator:SI 1 "aarch64_comparison_operator"

Re: [Ping] [PATCH, 8/10] aarch64: ccmp insn patterns

[Richard Henderson]

On 09/22/2014 11:45 PM, Zhenqiang Chen wrote:
> +(define_expand "cbranchcc4"
> +  [(set (pc) (if_then_else
> +	      (match_operator 0 "aarch64_comparison_operator"
> +	       [(match_operand 1 "cc_register" "")
> +	        (const_int 0)])
> +	      (label_ref (match_operand 3 "" ""))
> +	      (pc)))]
> +  ""
> +  " ")

Extra space.

> +(define_insn "*ccmp_and"
> +  [(set (match_operand 6 "ccmp_cc_register" "")
> +	(compare
> +	 (and:SI
> +	  (match_operator 4 "aarch64_comparison_operator"
> +	   [(match_operand 0 "ccmp_cc_register" "")
> +	    (match_operand 1 "aarch64_plus_operand" "")])
> +	  (match_operator 5 "aarch64_comparison_operator"
> +	   [(match_operand:GPI 2 "register_operand" "r,r,r")
> +	    (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn")]))
> +	 (const_int 0)))]
> +  ""
> +  {
> +    return aarch64_output_ccmp (operands, true, which_alternative);
> +  }
> +  [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
> +)
> +
> +(define_insn "*ccmp_ior"
> +  [(set (match_operand 6 "ccmp_cc_register" "")
> +	(compare
> +	 (ior:SI
> +	  (match_operator 4 "aarch64_comparison_operator"
> +	   [(match_operand 0 "ccmp_cc_register" "")
> +	    (match_operand 1 "aarch64_plus_operand" "")])
> +	  (match_operator 5 "aarch64_comparison_operator"
> +	   [(match_operand:GPI 2 "register_operand" "r,r,r")
> +	    (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn")]))
> +	 (const_int 0)))]
> +  ""
> +  {
> +    return aarch64_output_ccmp (operands, false, which_alternative);
> +  }
> +  [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]

Surely not aarch64_plus_operand for operand 1.  That's a comparison with the
flags register.  Surely (const_int 0) is the only valid operand there.

These could be combined with a code iterator, and thus there would be exactly
one call to aarch64_output_ccmp, and thus inlined.  Although...

It seems to me that you don't need a function call at all.  How about

AND
  "@
   ccmp\\t%<w>2, %<w>3, %K5, %m4
   ccmp\\t%<w>2, %<w>3, %K5, %m4
   ccmn\\t%<w>2, #%n3, %K5, %m4"

IOR
  "@
   ccmp\\t%<w>2, %<w>3, %k5, %M4
   ccmp\\t%<w>2, %<w>3, %k5, %M4
   ccmn\\t%<w>2, #%n3, %k5, %M4"

where 'k' and 'K' are new print_operand codes that output the nzcv (or its
inverse) integer for the comparison, much like 'm' and 'M' print the name of
the comparison.


r~

RE: [Ping] [PATCH, 8/10] aarch64: ccmp insn patterns

[Zhenqiang Chen]

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



> -----Original Message-----
> From: Richard Henderson [mailto:rth@redhat.com]
> Sent: Sunday, October 12, 2014 4:12 AM
> To: Zhenqiang Chen; gcc-patches@gcc.gnu.org
> Subject: Re: [Ping] [PATCH, 8/10] aarch64: ccmp insn patterns
> 
> On 09/22/2014 11:45 PM, Zhenqiang Chen wrote:
> > +(define_expand "cbranchcc4"
> > +  [(set (pc) (if_then_else
> > +	      (match_operator 0 "aarch64_comparison_operator"
> > +	       [(match_operand 1 "cc_register" "")
> > +	        (const_int 0)])
> > +	      (label_ref (match_operand 3 "" ""))
> > +	      (pc)))]
> > +  ""
> > +  " ")
> 
> Extra space.

Updated.
 
> > +(define_insn "*ccmp_and"
> > +  [(set (match_operand 6 "ccmp_cc_register" "")
> > +	(compare
> > +	 (and:SI
> > +	  (match_operator 4 "aarch64_comparison_operator"
> > +	   [(match_operand 0 "ccmp_cc_register" "")
> > +	    (match_operand 1 "aarch64_plus_operand" "")])
> > +	  (match_operator 5 "aarch64_comparison_operator"
> > +	   [(match_operand:GPI 2 "register_operand" "r,r,r")
> > +	    (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn")]))
> > +	 (const_int 0)))]
> > +  ""
> > +  {
> > +    return aarch64_output_ccmp (operands, true, which_alternative);
> > +  }
> > +  [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
> > +)
> > +
> > +(define_insn "*ccmp_ior"
> > +  [(set (match_operand 6 "ccmp_cc_register" "")
> > +	(compare
> > +	 (ior:SI
> > +	  (match_operator 4 "aarch64_comparison_operator"
> > +	   [(match_operand 0 "ccmp_cc_register" "")
> > +	    (match_operand 1 "aarch64_plus_operand" "")])
> > +	  (match_operator 5 "aarch64_comparison_operator"
> > +	   [(match_operand:GPI 2 "register_operand" "r,r,r")
> > +	    (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn")]))
> > +	 (const_int 0)))]
> > +  ""
> > +  {
> > +    return aarch64_output_ccmp (operands, false, which_alternative);
> > +  }
> > +  [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
> 
> Surely not aarch64_plus_operand for operand 1.  That's a comparison with
> the flags register.  Surely (const_int 0) is the only valid operand there.
> 
> These could be combined with a code iterator, and thus there would be
> exactly one call to aarch64_output_ccmp, and thus inlined.  Although...
> 
> It seems to me that you don't need a function call at all.  How about
> 
> AND
>   "@
>    ccmp\\t%<w>2, %<w>3, %K5, %m4
>    ccmp\\t%<w>2, %<w>3, %K5, %m4
>    ccmn\\t%<w>2, #%n3, %K5, %m4"
> 
> IOR
>   "@
>    ccmp\\t%<w>2, %<w>3, %k5, %M4
>    ccmp\\t%<w>2, %<w>3, %k5, %M4
>    ccmn\\t%<w>2, #%n3, %k5, %M4"
> 
> where 'k' and 'K' are new print_operand codes that output the nzcv (or its
> inverse) integer for the comparison, much like 'm' and 'M' print the name
of
> the comparison.

Updated with a bit change.

+   ccmp\\t%<w>2, %<w>3, %z5, %k4
+   ccmp\\t%<w>2, %<w>3, %z5, %k4
+   ccmn\\t%<w>2, #%n3, %z5, %k4"

z/Z is the k/K as you suggested. I can not reuse m/M since they are special
for cbranchcc (with CCMP). So I use new added k/K.

Thanks!
-Zhenqiang

[-- Attachment #2: 7-8-ccmp-patterns.patch --]
[-- Type: application/octet-stream, Size: 7001 bytes --]

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 1cf935a..3071aec 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3740,6 +3740,75 @@ bit_count (unsigned HOST_WIDE_INT value)
   return count;
 }
 
+/* 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)
+
+/* N Z C V flags for ccmp.  The first code is for AND op and the other
+   is for IOR op.  Indexed by AARCH64_COND_CODE.  */
+static const int aarch64_nzcv_codes[][2] =
+{
+  {AARCH64_CC_Z, 0}, /* EQ, Z == 1.  */
+  {0, AARCH64_CC_Z}, /* NE, Z == 0.  */
+  {AARCH64_CC_C, 0}, /* CS, C == 1.  */
+  {0, AARCH64_CC_C}, /* CC, C == 0.  */
+  {0, 0}, /* MI, not supported*/
+  {0, 0}, /* PL, not supported*/
+  {0, 0}, /* VS, not supported*/
+  {0, 0}, /* VC, not supported*/
+  {AARCH64_CC_C, AARCH64_CC_Z}, /* HI, C ==1 && Z == 0.  */
+  {AARCH64_CC_Z, AARCH64_CC_C}, /* LS, !(C == 1 && Z == 0).  */
+  {AARCH64_CC_N | AARCH64_CC_V, AARCH64_CC_N}, /* GE, N == V.  */
+  {AARCH64_CC_N, AARCH64_CC_N | AARCH64_CC_V}, /* LT, N != V.  */
+  {AARCH64_CC_N | AARCH64_CC_V, AARCH64_CC_Z}, /* GT, Z == 0 && N == V.  */
+  {AARCH64_CC_Z, AARCH64_CC_N | AARCH64_CC_V}, /* LE, !(Z == 0 && N == V).  */
+  {0, 0}, /* AL, not supported.  */ 
+  {0, 0}, /* NV, not supported.  */ 
+};
+
+static enum rtx_code
+aarch64_ccmp_mode_to_code (enum machine_mode mode)
+{
+  switch (mode)
+    {
+    case CC_DNEmode:
+      return NE;
+
+    case CC_DEQmode:
+      return EQ;
+
+    case CC_DLEmode:
+      return LE;
+
+    case CC_DGTmode:
+      return GT;
+
+    case CC_DLTmode:
+      return LT;
+
+    case CC_DGEmode:
+      return GE;
+
+    case CC_DLEUmode:
+      return LEU;
+
+    case CC_DGTUmode:
+      return GTU;
+
+    case CC_DLTUmode:
+      return LTU;
+
+    case CC_DGEUmode:
+      return GEU;
+
+    default:
+      gcc_unreachable ();
+    }
+}
+
+
 void
 aarch64_print_operand (FILE *f, rtx x, char code)
 {
@@ -4104,6 +4173,81 @@ aarch64_print_operand (FILE *f, rtx x, char code)
       output_addr_const (asm_out_file, x);
       break;
 
+    case 'k':
+      {
+	int cond_code;
+	rtx op0 = XEXP (x, 0);
+	enum rtx_code mode_code;
+	/* Print a condition (eq, ne, etc) of ccmp.  */
+
+	if (!COMPARISON_P (x) || !ccmp_cc_register (op0, GET_MODE (op0)))
+	  {
+	    output_operand_lossage ("invalid operand for '%%%c'", code);
+	    return;
+	  }
+
+	mode_code = aarch64_ccmp_mode_to_code (GET_MODE (op0));
+	cond_code = aarch64_get_condition_code_1 (CCmode, mode_code);
+	gcc_assert (cond_code >= 0);
+	fputs (aarch64_condition_codes[cond_code], f);
+      }
+      break;
+
+    case 'K':
+      {
+	int cond_code;
+	rtx op0 = XEXP (x, 0);
+	enum rtx_code mode_code;
+	/* Print a condition (eq, ne, etc) of ccmp.  */
+
+	if (!COMPARISON_P (x) || !ccmp_cc_register (op0, GET_MODE (op0)))
+	  {
+	    output_operand_lossage ("invalid operand for '%%%c'", code);
+	    return;
+	  }
+
+	mode_code = aarch64_ccmp_mode_to_code (GET_MODE (op0));
+	cond_code = aarch64_get_condition_code_1 (CCmode, mode_code);
+	gcc_assert (cond_code >= 0);
+	fputs (aarch64_condition_codes[AARCH64_INVERSE_CONDITION_CODE
+				       (cond_code)], f);
+      }
+      break;
+
+    case 'Z':
+      {
+	int cond_code;
+	/* Print nzcv.  */
+
+	if (!COMPARISON_P (x))
+	  {
+	    output_operand_lossage ("invalid operand for '%%%c'", code);
+	    return;
+	  }
+
+	cond_code = aarch64_get_condition_code_1 (CCmode, GET_CODE (x));
+	gcc_assert (cond_code >= 0);
+	asm_fprintf (f, "%d", aarch64_nzcv_codes[cond_code][0]);
+      }
+      break;
+
+    case 'z':
+      {
+	int cond_code;
+	/* Print nzcv.  */
+
+	if (!COMPARISON_P (x))
+	  {
+	    output_operand_lossage ("invalid operand for '%%%c'", code);
+	    return;
+	  }
+
+	cond_code = aarch64_get_condition_code_1 (CCmode, GET_CODE (x));
+	gcc_assert (cond_code >= 0);
+	asm_fprintf (f, "%d", aarch64_nzcv_codes[cond_code][1]);
+      }
+      break;
+
     default:
       output_operand_lossage ("invalid operand prefix '%%%c'", code);
       return;
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 74b554e..b8b5b60 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -239,6 +239,54 @@
   "
 )
 
+(define_expand "cbranchcc4"
+  [(set (pc) (if_then_else
+	      (match_operator 0 "aarch64_comparison_operator"
+	       [(match_operand 1 "cc_register" "")
+	        (match_operand 2 "const0_operand")])
+	      (label_ref (match_operand 3 "" ""))
+	      (pc)))]
+  ""
+  "")
+
+(define_insn "*ccmp_and"
+  [(set (match_operand 1 "ccmp_cc_register" "")
+	(compare
+	 (and:SI
+	  (match_operator 4 "aarch64_comparison_operator"
+	   [(match_operand 0 "ccmp_cc_register" "")
+	    (const_int 0)])
+	  (match_operator 5 "aarch64_comparison_operator"
+	   [(match_operand:GPI 2 "register_operand" "r,r,r")
+	    (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn")]))
+	 (const_int 0)))]
+  ""
+  "@
+   ccmp\\t%<w>2, %<w>3, %z5, %k4
+   ccmp\\t%<w>2, %<w>3, %z5, %k4
+   ccmn\\t%<w>2, #%n3, %z5, %k4"
+  [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
+)
+
+(define_insn "*ccmp_ior"
+  [(set (match_operand 1 "ccmp_cc_register" "")
+	(compare
+	 (ior:SI
+	  (match_operator 4 "aarch64_comparison_operator"
+	   [(match_operand 0 "ccmp_cc_register" "")
+	    (const_int 0)])
+	  (match_operator 5 "aarch64_comparison_operator"
+	   [(match_operand:GPI 2 "register_operand" "r,r,r")
+	    (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn")]))
+	 (const_int 0)))]
+  ""
+  "@
+   ccmp\\t%<w>2, %<w>3, %Z5, %K4
+   ccmp\\t%<w>2, %<w>3, %Z5, %K4
+   ccmn\\t%<w>2, #%n3, %Z5, %K4"
+  [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
+)
+
 (define_insn "*condjump"
   [(set (pc) (if_then_else (match_operator 0 "aarch64_comparison_operator"
 			    [(match_operand 1 "cc_register" "") (const_int 0)])
@@ -2395,6 +2443,18 @@
   "
 )
 
+(define_expand "cstorecc4"
+  [(set (match_operand:SI 0 "register_operand")
+       (match_operator 1 "aarch64_comparison_operator"
+        [(match_operand 2 "ccmp_cc_register")
+         (match_operand 3 "const0_operand")]))]
+  ""
+"{
+  emit_insn (gen_rtx_SET (SImode, operands[0], operands[1]));
+  DONE;
+}")
+
+
 (define_expand "cstore<mode>4"
   [(set (match_operand:SI 0 "register_operand" "")
 	(match_operator:SI 1 "aarch64_comparison_operator"
diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md
index f6fe58c..7129203 100644
--- a/gcc/config/aarch64/predicates.md
+++ b/gcc/config/aarch64/predicates.md
@@ -30,6 +30,11 @@
   (ior (match_code "symbol_ref")
        (match_operand 0 "register_operand")))
 
+;; Return true if OP a (const_int 0) operand.
+(define_predicate "const0_operand"
+  (and (match_code "const_int, const_double")
+       (match_test "op == CONST0_RTX (mode)")))
+
 (define_predicate "aarch64_ccmp_immediate"
   (and (match_code "const_int")
        (match_test "IN_RANGE (INTVAL (op), -31, 31)")))

Re: [Ping] [PATCH, 8/10] aarch64: ccmp insn patterns

[Richard Henderson]

On 10/27/2014 12:49 AM, Zhenqiang Chen wrote:
> +  {AARCH64_CC_Z, 0}, /* EQ, Z == 1.  */
> +  {0, AARCH64_CC_Z}, /* NE, Z == 0.  */
> +  {AARCH64_CC_C, 0}, /* CS, C == 1.  */
> +  {0, AARCH64_CC_C}, /* CC, C == 0.  */
> +  {0, 0}, /* MI, not supported*/
> +  {0, 0}, /* PL, not supported*/
> +  {0, 0}, /* VS, not supported*/
> +  {0, 0}, /* VC, not supported*/

Why not go ahead and fill out the table?  You know what needs to go in these
slots, after all.

> +  {AARCH64_CC_C, AARCH64_CC_Z}, /* HI, C ==1 && Z == 0.  */
> +  {AARCH64_CC_Z, AARCH64_CC_C}, /* LS, !(C == 1 && Z == 0).  */
> +  {AARCH64_CC_N | AARCH64_CC_V, AARCH64_CC_N}, /* GE, N == V.  */
> +  {AARCH64_CC_N, AARCH64_CC_N | AARCH64_CC_V}, /* LT, N != V.  */
> +  {AARCH64_CC_N | AARCH64_CC_V, AARCH64_CC_Z}, /* GT, Z == 0 && N == V.  */
> +  {AARCH64_CC_Z, AARCH64_CC_N | AARCH64_CC_V}, /* LE, !(Z == 0 && N == V).  */

Perhaps it's me, but does it make things clearer to reduce these?
That is, for the compound conditions, we need not make both sub-conditions be
false, only one of them.  E.g.

  {AARCH64_CC_C, 0} 		/* HI, C ==1 && Z == 0.  */
  {0, AARCH64_CC_C}		/* LS, !(C ==1 && Z == 0) */
  {0, AARCH64_CC_V}		/* GE, N == V */
  {AARCH64_CC_V, 0}		/* LT, N != V */
  {0, AARCH64_CC_Z}		/* GT, Z == 0 && N == V */
  {AARCH64_CC_Z, 0}		/* LE, !(Z == 0 && N == V) */

At which point it becomes blindingly obvious that while we can't compress the
table with ~nczv, we can index it with reverse_comparison instead.

> +    case 'k':
> +      {
> +	int cond_code;
> +	rtx op0 = XEXP (x, 0);
> +	enum rtx_code mode_code;
> +	/* Print a condition (eq, ne, etc) of ccmp.  */
> +
> +	if (!COMPARISON_P (x) || !ccmp_cc_register (op0, GET_MODE (op0)))
> +	  {
> +	    output_operand_lossage ("invalid operand for '%%%c'", code);
> +	    return;
> +	  }
> +
> +	mode_code = aarch64_ccmp_mode_to_code (GET_MODE (op0));
> +	cond_code = aarch64_get_condition_code_1 (CCmode, mode_code);
> +	gcc_assert (cond_code >= 0);
> +	fputs (aarch64_condition_codes[cond_code], f);
> +      }

Is there a branch with all the patches applied?  I can't look back at the
modified aarch64_get_condition_code_1, but off-hand I can't think of why %m/%M
wouldn't work.  Surely

  aarch64_get_condition_code_1 (GET_MODE (op0), GET_CODE (x))

will yield the correct cond_code.  If it didn't, then surely branches wouldn't
work at all.

These are not some magic new kind of conditions; they're exactly the same.


r~

RE: [Ping] [PATCH, 8/10] aarch64: ccmp insn patterns

[Zhenqiang Chen]

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


> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Richard Henderson
> Sent: Monday, October 27, 2014 11:47 PM
> To: Zhenqiang Chen
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [Ping] [PATCH, 8/10] aarch64: ccmp insn patterns
> 
> On 10/27/2014 12:49 AM, Zhenqiang Chen wrote:
> > +  {AARCH64_CC_Z, 0}, /* EQ, Z == 1.  */  {0, AARCH64_CC_Z}, /* NE, Z
> > + == 0.  */  {AARCH64_CC_C, 0}, /* CS, C == 1.  */  {0, AARCH64_CC_C},
> > + /* CC, C == 0.  */  {0, 0}, /* MI, not supported*/  {0, 0}, /* PL,
> > + not supported*/  {0, 0}, /* VS, not supported*/  {0, 0}, /* VC, not
> > + supported*/
> 
> Why not go ahead and fill out the table?  You know what needs to go in
these
> slots, after all.

Updated.

> > +  {AARCH64_CC_C, AARCH64_CC_Z}, /* HI, C ==1 && Z == 0.  */
> > + {AARCH64_CC_Z, AARCH64_CC_C}, /* LS, !(C == 1 && Z == 0).  */
> > + {AARCH64_CC_N | AARCH64_CC_V, AARCH64_CC_N}, /* GE, N == V.  */
> > + {AARCH64_CC_N, AARCH64_CC_N | AARCH64_CC_V}, /* LT, N != V.  */
> > + {AARCH64_CC_N | AARCH64_CC_V, AARCH64_CC_Z}, /* GT, Z == 0 && N
> ==
> > + V.  */  {AARCH64_CC_Z, AARCH64_CC_N | AARCH64_CC_V}, /* LE, !(Z ==
> 0
> > + && N == V).  */
> 
> Perhaps it's me, but does it make things clearer to reduce these?
> That is, for the compound conditions, we need not make both sub-conditions
> be false, only one of them.  E.g.
> 
>   {AARCH64_CC_C, 0} 		/* HI, C ==1 && Z == 0.  */
>   {0, AARCH64_CC_C}		/* LS, !(C ==1 && Z == 0) */
>   {0, AARCH64_CC_V}		/* GE, N == V */
>   {AARCH64_CC_V, 0}		/* LT, N != V */
>   {0, AARCH64_CC_Z}		/* GT, Z == 0 && N == V */
>   {AARCH64_CC_Z, 0}		/* LE, !(Z == 0 && N == V) */
> 
> At which point it becomes blindingly obvious that while we can't compress
> the table with ~nczv, we can index it with reverse_comparison instead.

Updated.

> > +    case 'k':
> > +      {
> > +	int cond_code;
> > +	rtx op0 = XEXP (x, 0);
> > +	enum rtx_code mode_code;
> > +	/* Print a condition (eq, ne, etc) of ccmp.  */
> > +
> > +	if (!COMPARISON_P (x) || !ccmp_cc_register (op0, GET_MODE
> (op0)))
> > +	  {
> > +	    output_operand_lossage ("invalid operand for '%%%c'", code);
> > +	    return;
> > +	  }
> > +
> > +	mode_code = aarch64_ccmp_mode_to_code (GET_MODE (op0));
> > +	cond_code = aarch64_get_condition_code_1 (CCmode, mode_code);
> > +	gcc_assert (cond_code >= 0);
> > +	fputs (aarch64_condition_codes[cond_code], f);
> > +      }
> 
> Is there a branch with all the patches applied?  I can't look back at the
> modified aarch64_get_condition_code_1, but off-hand I can't think of
> why %m/%M wouldn't work.  Surely

It's my fault. %m/%M work well in the new patch.

And I add a check 

aarch64_ccmp_mode_to_code (GET_MODE (operands[1])) == GET_CODE (operands[5])

on the patterns to make sure that the compare and CC mode are aligned.

Thanks!
-Zhenqiang

 
>   aarch64_get_condition_code_1 (GET_MODE (op0), GET_CODE (x))
> 
> will yield the correct cond_code.  If it didn't, then surely branches
wouldn't
> work at all.
> 
> These are not some magic new kind of conditions; they're exactly the same.
> 
> 
> r~

[-- Attachment #2: 7-8-ccmp-patterns.patch --]
[-- Type: application/octet-stream, Size: 7096 bytes --]

diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index c57a467..2334a30 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -313,4 +313,5 @@ extern void aarch64_final_prescan_insn (rtx_insn *);
 extern bool
 aarch64_expand_vec_perm_const (rtx target, rtx op0, rtx op1, rtx sel);
 void aarch64_atomic_assign_expand_fenv (tree *, tree *, tree *);
+int aarch64_ccmp_mode_to_code (enum machine_mode mode);
 #endif /* GCC_AARCH64_PROTOS_H */
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 9799957..047f510 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3752,6 +3752,75 @@ bit_count (unsigned HOST_WIDE_INT value)
   return count;
 }
 
+/* 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)
+
+/* N Z C V flags for ccmp.  The first code is for AND op and the other
+   is for IOR op.  Indexed by AARCH64_COND_CODE.  */
+static const int aarch64_nzcv_codes[][2] =
+{
+  {AARCH64_CC_Z, 0}, /* EQ, Z == 1.  */
+  {0, AARCH64_CC_Z}, /* NE, Z == 0.  */
+  {AARCH64_CC_C, 0}, /* CS, C == 1.  */
+  {0, AARCH64_CC_C}, /* CC, C == 0.  */
+  {AARCH64_CC_N, 0}, /* MI, N == 1.  */
+  {0, AARCH64_CC_N}, /* PL, N == 0.  */
+  {AARCH64_CC_V, 0}, /* VS, V == 1.  */
+  {0, AARCH64_CC_V}, /* VC, V == 0.  */
+  {AARCH64_CC_C, 0}, /* HI, C ==1 && Z == 0.  */
+  {0, AARCH64_CC_C}, /* LS, !(C == 1 && Z == 0).  */
+  {0, AARCH64_CC_V}, /* GE, N == V.  */
+  {AARCH64_CC_V, 0}, /* LT, N != V.  */
+  {0, AARCH64_CC_Z}, /* GT, Z == 0 && N == V.  */
+  {AARCH64_CC_Z, 0}, /* LE, !(Z == 0 && N == V).  */
+  {0, 0}, /* AL, Any.  */
+  {0, 0}, /* NV, Any.  */
+};
+
+int
+aarch64_ccmp_mode_to_code (enum machine_mode mode)
+{
+  switch (mode)
+    {
+    case CC_DNEmode:
+      return NE;
+
+    case CC_DEQmode:
+      return EQ;
+
+    case CC_DLEmode:
+      return LE;
+
+    case CC_DGTmode:
+      return GT;
+
+    case CC_DLTmode:
+      return LT;
+
+    case CC_DGEmode:
+      return GE;
+
+    case CC_DLEUmode:
+      return LEU;
+
+    case CC_DGTUmode:
+      return GTU;
+
+    case CC_DLTUmode:
+      return LTU;
+
+    case CC_DGEUmode:
+      return GEU;
+
+    default:
+      gcc_unreachable ();
+    }
+}
+
+
 void
 aarch64_print_operand (FILE *f, rtx x, char code)
 {
@@ -4116,6 +4185,40 @@ aarch64_print_operand (FILE *f, rtx x, char code)
       output_addr_const (asm_out_file, x);
       break;
 
+    case 'K':
+      {
+	int cond_code;
+	/* Print nzcv.  */
+
+	if (!COMPARISON_P (x))
+	  {
+	    output_operand_lossage ("invalid operand for '%%%c'", code);
+	    return;
+	  }
+
+	cond_code = aarch64_get_condition_code_1 (CCmode, GET_CODE (x));
+	gcc_assert (cond_code >= 0);
+	asm_fprintf (f, "%d", aarch64_nzcv_codes[cond_code][0]);
+      }
+      break;
+
+    case 'k':
+      {
+	int cond_code;
+	/* Print nzcv.  */
+
+	if (!COMPARISON_P (x))
+	  {
+	    output_operand_lossage ("invalid operand for '%%%c'", code);
+	    return;
+	  }
+
+	cond_code = aarch64_get_condition_code_1 (CCmode, GET_CODE (x));
+	gcc_assert (cond_code >= 0);
+	asm_fprintf (f, "%d", aarch64_nzcv_codes[cond_code][1]);
+      }
+      break;
+
     default:
       output_operand_lossage ("invalid operand prefix '%%%c'", code);
       return;
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 341c26f..df2f591 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -245,6 +245,54 @@
   "
 )
 
+(define_expand "cbranchcc4"
+  [(set (pc) (if_then_else
+	      (match_operator 0 "aarch64_comparison_operator"
+	       [(match_operand 1 "cc_register" "")
+	        (match_operand 2 "const0_operand")])
+	      (label_ref (match_operand 3 "" ""))
+	      (pc)))]
+  ""
+  "")
+
+(define_insn "*ccmp_and"
+  [(set (match_operand 1 "ccmp_cc_register" "")
+	(compare
+	 (and:SI
+	  (match_operator 4 "aarch64_comparison_operator"
+	   [(match_operand 0 "ccmp_cc_register" "")
+	    (const_int 0)])
+	  (match_operator 5 "aarch64_comparison_operator"
+	   [(match_operand:GPI 2 "register_operand" "r,r,r")
+	    (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn")]))
+	 (const_int 0)))]
+  "aarch64_ccmp_mode_to_code (GET_MODE (operands[1])) == GET_CODE (operands[5])"
+  "@
+   ccmp\\t%<w>2, %<w>3, %k5, %m4
+   ccmp\\t%<w>2, %<w>3, %k5, %m4
+   ccmn\\t%<w>2, #%n3, %k5, %m4"
+  [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
+)
+
+(define_insn "*ccmp_ior"
+  [(set (match_operand 1 "ccmp_cc_register" "")
+	(compare
+	 (ior:SI
+	  (match_operator 4 "aarch64_comparison_operator"
+	   [(match_operand 0 "ccmp_cc_register" "")
+	    (const_int 0)])
+	  (match_operator 5 "aarch64_comparison_operator"
+	   [(match_operand:GPI 2 "register_operand" "r,r,r")
+	    (match_operand:GPI 3 "aarch64_ccmp_operand" "r,Uss,Usn")]))
+	 (const_int 0)))]
+  "aarch64_ccmp_mode_to_code (GET_MODE (operands[1])) == GET_CODE (operands[5])"
+  "@
+   ccmp\\t%<w>2, %<w>3, %K5, %M4
+   ccmp\\t%<w>2, %<w>3, %K5, %M4
+   ccmn\\t%<w>2, #%n3, %K5, %M4"
+  [(set_attr "type" "alus_sreg,alus_imm,alus_imm")]
+)
+
 (define_insn "*condjump"
   [(set (pc) (if_then_else (match_operator 0 "aarch64_comparison_operator"
 			    [(match_operand 1 "cc_register" "") (const_int 0)])
@@ -2401,6 +2449,18 @@
   "
 )
 
+(define_expand "cstorecc4"
+  [(set (match_operand:SI 0 "register_operand")
+       (match_operator 1 "aarch64_comparison_operator"
+        [(match_operand 2 "ccmp_cc_register")
+         (match_operand 3 "const0_operand")]))]
+  ""
+"{
+  emit_insn (gen_rtx_SET (SImode, operands[0], operands[1]));
+  DONE;
+}")
+
+
 (define_expand "cstore<mode>4"
   [(set (match_operand:SI 0 "register_operand" "")
 	(match_operator:SI 1 "aarch64_comparison_operator"
@@ -2549,15 +2609,19 @@
 			   (match_operand:ALLI 3 "register_operand" "")))]
   ""
   {
-    rtx ccreg;
     enum rtx_code code = GET_CODE (operands[1]);
 
     if (code == UNEQ || code == LTGT)
       FAIL;
 
-    ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
-				  XEXP (operands[1], 1));
-    operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
+    if (!ccmp_cc_register (XEXP (operands[1], 0),
+			   GET_MODE (XEXP (operands[1], 0))))
+      {
+	rtx ccreg;
+	ccreg = aarch64_gen_compare_reg (code, XEXP (operands[1], 0),
+					 XEXP (operands[1], 1));
+	operands[1] = gen_rtx_fmt_ee (code, VOIDmode, ccreg, const0_rtx);
+      }
   }
 )
 
diff --git a/gcc/config/aarch64/predicates.md b/gcc/config/aarch64/predicates.md
index f6fe58c..7129203 100644
--- a/gcc/config/aarch64/predicates.md
+++ b/gcc/config/aarch64/predicates.md
@@ -30,6 +30,11 @@
   (ior (match_code "symbol_ref")
        (match_operand 0 "register_operand")))
 
+;; Return true if OP a (const_int 0) operand.
+(define_predicate "const0_operand"
+  (and (match_code "const_int, const_double")
+       (match_test "op == CONST0_RTX (mode)")))
+
 (define_predicate "aarch64_ccmp_immediate"
   (and (match_code "const_int")
        (match_test "IN_RANGE (INTVAL (op), -31, 31)")))

Re: [Ping] [PATCH, 8/10] aarch64: ccmp insn patterns

[Richard Henderson]

On 10/29/2014 03:37 AM, Zhenqiang Chen wrote:
> It's my fault. %m/%M work well in the new patch.
> 
> And I add a check 
> 
> aarch64_ccmp_mode_to_code (GET_MODE (operands[1])) == GET_CODE (operands[5])
> 
> on the patterns to make sure that the compare and CC mode are aligned.

Looks good.


r~