diff mbox

[RFC,4.8] Magic matching for flags clobbering and setting

Message ID 4F35B27A.9020907@redhat.com
State New
Headers show

Commit Message

Richard Henderson Feb. 11, 2012, 12:12 a.m. UTC
Seeing as how Uros is starting to go down the path of cleaning up the
flags handling for x86, I thought I'd go ahead and knock up the idea
that I've been tossing around to help automate the process of building
patterns that match both clobbering the flags and setting the flags to
a comparison.

This is far from complete, but it at least shows the direction.

What I know is missing off the top of my head are:

 (0) Documentation in some .texi file; atm there's only what's in rtl.def.

 (1) Generate (clobber (reg flags)) from genemit, should this construct
     be used in a named insn pattern.

 (2) Can't be usefully used with define_insn_and_split, and no way to tell.
     This problem should simply be documented in the .texi file as user error.

 (3) Can't be used for x86 add patterns, as the "clobber" version wants the
     freedom to use "lea" and the "set flags" version cannot.  And there are
     different sets of constraints if lea may be used or not.

     What would be nice, however, is exposing the targetm.cc_modes_compatible
     thing in such a way that the x86 add patterns could use that, for the
     separate insn that does do the set flags.

     Exposing the targetm.cc_modes_compatible thing separately might also
     clean up some of the evil magic in genrecog.c too.

Comments?


r~
commit c01bac2f6bf0817d03849c912165986176e82985
Author: Richard Henderson <rth@redhat.com>
Date:   Fri Feb 10 15:46:11 2012 -0800

    Introduce 'v' rtx format.
commit 4b41537543c3c918fb7179daef06f07716305c86
Author: Richard Henderson <rth@redhat.com>
Date:   Fri Feb 10 15:47:54 2012 -0800

    Introduce MATCH_FLAGS.

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 770925f..85ea198 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -3523,8 +3523,8 @@ insn-preds.o : insn-preds.c $(CONFIG_H) $(SYSTEM_H) coretypes.h		\
   $(DIAGNOSTIC_CORE_H) reload.h $(REGS_H) tm-constrs.h
 insn-recog.o : insn-recog.c $(CONFIG_H) $(SYSTEM_H) coretypes.h		\
   $(TM_H) $(RTL_H) insn-config.h $(RECOG_H) output.h $(FLAGS_H)		\
-  $(FUNCTION_H) hard-reg-set.h $(RESOURCE_H) $(TM_P_H) $(DIAGNOSTIC_CORE_H)	\
-  reload.h $(REGS_H) tm-constrs.h
+  $(FUNCTION_H) hard-reg-set.h $(RESOURCE_H) $(TM_P_H) $(DIAGNOSTIC_CORE_H) \
+  reload.h $(REGS_H) tm-constrs.h $(TARGET_H)
 
 # For each of the files generated by running a generator program over
 # the machine description, the following static pattern rules run the
@@ -3917,7 +3917,7 @@ build/genpeep.o : genpeep.c $(RTL_BASE_H) $(BCONFIG_H) $(SYSTEM_H)	\
 build/genpreds.o : genpreds.c $(RTL_BASE_H) $(BCONFIG_H) $(SYSTEM_H)	\
   coretypes.h $(GTM_H) errors.h $(READ_MD_H) gensupport.h $(OBSTACK_H)
 build/genrecog.o : genrecog.c $(RTL_BASE_H) $(BCONFIG_H) $(SYSTEM_H)	\
-  coretypes.h $(GTM_H) errors.h $(READ_MD_H) gensupport.h
+  coretypes.h $(GTM_H) errors.h $(READ_MD_H) gensupport.h genrtl.h
 build/genhooks.o : genhooks.c $(TARGET_DEF) $(C_TARGET_DEF)		\
   $(COMMON_TARGET_DEF) $(BCONFIG_H) $(SYSTEM_H) errors.h
 
diff --git a/gcc/genrecog.c b/gcc/genrecog.c
index 0d8be8f..bff7f66 100644
--- a/gcc/genrecog.c
+++ b/gcc/genrecog.c
@@ -58,10 +58,16 @@
 #include "errors.h"
 #include "read-md.h"
 #include "gensupport.h"
+#include "genrtl.h"
 
 #define OUTPUT_LABEL(INDENT_STRING, LABEL_NUMBER) \
   printf("%sL%d: ATTRIBUTE_UNUSED_LABEL\n", (INDENT_STRING), (LABEL_NUMBER))
 
+/* Encode ways to adjust comparisons vs modes, for MATCH_FLAGS.  */
+#define MODE_CC_ANY	(enum machine_mode)0x80
+#define MODE_CC_DEST	(enum machine_mode)0x81
+#define MODE_CC_COMPAT	0x80
+
 /* Ways of obtaining an rtx to be tested.  */
 enum position_type {
   /* PATTERN (peep2_next_insn (ARG)).  */
@@ -119,7 +125,8 @@ struct decision_head
 enum decision_type
 {
   DT_num_insns,
-  DT_mode, DT_code, DT_veclen,
+  DT_mode, DT_mode_cc_any, DT_mode_cc_compat, DT_mode_dest,
+  DT_code, DT_veclen,
   DT_elt_zero_int, DT_elt_one_int, DT_elt_zero_wide, DT_elt_zero_wide_safe,
   DT_const_int,
   DT_veclen_ge, DT_dup, DT_pred, DT_c_test,
@@ -435,6 +442,7 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int set_code)
     {
     case MATCH_SCRATCH:
       return;
+
     case MATCH_DUP:
     case MATCH_OP_DUP:
     case MATCH_PAR_DUP:
@@ -443,6 +451,12 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int set_code)
 			 "operand %i duplicated before defined",
 			 XINT (pattern, 0));
       break;
+
+    case MATCH_FLAGS:
+      error_with_line (pattern_lineno,
+		       "match_flags used in an invalid context");
+      break;
+
     case MATCH_OPERAND:
     case MATCH_OPERATOR:
       {
@@ -577,7 +591,10 @@ validate_pattern (rtx pattern, rtx insn, rtx set, int set_code)
 
         /* The operands of a SET must have the same mode unless one
 	   is VOIDmode.  */
-        else if (dmode != VOIDmode && smode != VOIDmode && dmode != smode)
+        else if (dmode != VOIDmode
+		 && smode != VOIDmode
+	         && smode != MODE_CC_DEST
+		 && dmode != smode)
 	  error_with_line (pattern_lineno,
 			   "mode mismatch in set: %smode vs %smode",
 			   GET_MODE_NAME (dmode), GET_MODE_NAME (smode));
@@ -957,8 +974,19 @@ add_to_sequence (rtx pattern, struct decision_head *last,
 
   if (mode != VOIDmode)
     {
+      enum decision_type dt = DT_mode;
+      if (mode == MODE_CC_ANY)
+	dt = DT_mode_cc_any;
+      else if (mode == MODE_CC_DEST)
+	dt = DT_mode_dest;
+      else if (mode & MODE_CC_COMPAT)
+	{
+	  dt = DT_mode_cc_compat;
+	  mode = (enum machine_mode)(mode & ~MODE_CC_COMPAT);
+	}
+      
       place = &this_decision->tests;
-      test = new_decision_test (DT_mode, &place);
+      test = new_decision_test (dt, &place);
       test->u.mode = mode;
     }
 
@@ -986,7 +1014,11 @@ maybe_both_true_2 (struct decision_test *d1, struct decision_test *d2)
 	    return -1;
 
 	case DT_mode:
+	case DT_mode_cc_any:
+	case DT_mode_cc_compat:
 	  return d1->u.mode == d2->u.mode;
+	case DT_mode_dest:
+	  return -1;
 
 	case DT_code:
 	  return d1->u.code == d2->u.code;
@@ -1187,6 +1219,9 @@ nodes_identical_1 (struct decision_test *d1, struct decision_test *d2)
       return d1->u.num_insns == d2->u.num_insns;
 
     case DT_mode:
+    case DT_mode_cc_any:
+    case DT_mode_cc_compat:
+    case DT_mode_dest:
       return d1->u.mode == d2->u.mode;
 
     case DT_code:
@@ -1863,6 +1898,19 @@ write_cond (struct decision_test *p, int depth,
       printf ("GET_MODE (x%d) == %smode", depth, GET_MODE_NAME (p->u.mode));
       break;
 
+    case DT_mode_cc_any:
+      printf ("GET_MODE_CLASS (GET_MODE (x%d)) == MODE_CC", depth);
+      break;
+
+    case DT_mode_cc_compat:
+      printf ("targetm.cc_modes_compatible (GET_MODE (x%d), %smode) == %smode",
+	      depth, GET_MODE_NAME (p->u.mode), GET_MODE_NAME (p->u.mode));
+      break;
+
+    case DT_mode_dest:
+      printf ("GET_MODE (x%d) == GET_MODE (SET_DEST (x%d))", depth, depth - 1);
+      break;
+
     case DT_code:
       printf ("GET_CODE (x%d) == ", depth);
       print_code (p->u.code);
@@ -2273,6 +2321,7 @@ write_header (void)
 #include \"reload.h\"\n\
 #include \"regs.h\"\n\
 #include \"tm-constrs.h\"\n\
+#include \"target.h\"\n\
 \n");
 
   puts ("\n\
@@ -2480,6 +2529,114 @@ make_insn_sequence (rtx insn, enum routine_type type)
   return head;
 }
 
+static rtx
+gen_compare_for_match_flags (rtx x)
+{
+  RTX_CODE code = GET_CODE (x);
+  const char *fmt;
+  int i, len;
+
+  switch (code)
+    {
+    case MATCH_OPERAND:
+    case MATCH_OPERATOR:
+    case MATCH_PARALLEL:
+      return gen_rtx_MATCH_DUP (VOIDmode, XINT (x, 0));
+
+    default:
+      x = shallow_copy_rtx (x);
+      break;
+    }
+
+  fmt = GET_RTX_FORMAT (code);
+  len = GET_RTX_LENGTH (code);
+  for (i = 0; i < len; ++i)
+    switch (fmt[i])
+      {
+      case 'e':
+	XEXP (x, i) = gen_compare_for_match_flags (XEXP (x, i));
+	break;
+
+      case 'E':
+	{
+	  int j, vlen = XVECLEN (x, i);
+	  rtvec vec = rtvec_alloc (vlen);
+	  for (j = 0; j < vlen; ++j)
+	    RTVEC_ELT (vec, j)
+	      = gen_compare_for_match_flags (XVECEXP (x, i, j));
+	  XVEC (x, i) = vec;
+	}
+	break;
+      }
+
+  return x;
+}
+
+static struct decision_head
+maybe_handle_match_flags (rtx insn)
+{
+  int mf_index = -1;
+  int i, n = XVECLEN (insn, 1);
+  rtx mf, mf_reg, mf_cmp;
+  enum machine_mode mf_mode;
+  struct decision_head h1, h2;
+
+  for (i = 0; i < n; ++i)
+    if (GET_CODE (XVECEXP (insn, 1, i)) == MATCH_FLAGS)
+      mf_index = i;
+
+  /* Either match_flags not used, or used invalidly.  The error message
+     for the later case will be emitted by validate_pattern.  */
+  if (mf_index <= 0)
+    return make_insn_sequence (insn, RECOG);
+
+  /* Assert that the first argument is of the right form.  */
+  mf = XVECEXP (insn, 1, mf_index);
+  mf_reg = XEXP (mf, 0);
+  if (GET_CODE (mf_reg) != REG)
+    error_with_line (pattern_lineno,
+		     "match_flags first operand must be a register");
+
+  mf_mode = GET_MODE (mf_reg);
+  if (GET_MODE_CLASS (mf_mode) != MODE_CC)
+    error_with_line (pattern_lineno,
+		     "match_flags first operand must be MODE_CC");
+
+  /* First match the insn with the clobber.  */
+  PUT_MODE (mf_reg, MODE_CC_ANY);
+  XVECEXP (insn, 1, mf_index) = gen_rtx_CLOBBER (VOIDmode, mf_reg);
+  h1 = make_insn_sequence (insn, RECOG);
+
+  /* Second, match the insn with the comparison.  */
+  PUT_MODE (mf_reg, (enum machine_mode)(mf_mode | MODE_CC_COMPAT));
+
+  /* Generate the default comparison, if none supplied.  */
+  mf_cmp = XEXP (mf, 1);
+  if (mf_cmp == NULL)
+    {
+      rtx first = XVECEXP (insn, 1, 0);
+      if (GET_CODE (first) != SET)
+	{
+	  error_with_line
+	    (pattern_lineno,
+	     "first element of parallel not a set for match_flags");
+	  first = GEN_INT (0);
+        }
+      else
+	first = gen_compare_for_match_flags (SET_SRC (first));
+
+      mf_cmp = gen_rtx_COMPARE (MODE_CC_DEST, first, GEN_INT (0));
+    }
+  else
+    PUT_MODE (mf_cmp, MODE_CC_DEST);
+
+  XVECEXP (insn, 1, mf_index) = gen_rtx_SET (VOIDmode, mf_reg, mf_cmp);
+  h2 = make_insn_sequence (insn, RECOG);
+
+  merge_trees (&h1, &h2);
+  return h1;
+}
+
 static void
 process_tree (struct decision_head *head, enum routine_type subroutine_type)
 {
@@ -2540,7 +2697,7 @@ main (int argc, char **argv)
       switch (GET_CODE (desc))
 	{
 	case DEFINE_INSN:
-	  h = make_insn_sequence (desc, RECOG);
+	  h = maybe_handle_match_flags (desc);
 	  merge_trees (&recog_tree, &h);
 	  break;
 
@@ -2582,6 +2739,15 @@ debug_decision_2 (struct decision_test *test)
     case DT_mode:
       fprintf (stderr, "mode=%s", GET_MODE_NAME (test->u.mode));
       break;
+    case DT_mode_cc_any:
+      fprintf (stderr, "mode_cc_any");
+      break;
+    case DT_mode_cc_compat:
+      fprintf (stderr, "mode_cc_compat=%s", GET_MODE_NAME (test->u.mode));
+      break;
+    case DT_mode_dest:
+      fprintf (stderr, "mode_dest");
+      break;
     case DT_code:
       fprintf (stderr, "code=%s", GET_RTX_NAME (test->u.code));
       break;
diff --git a/gcc/rtl.def b/gcc/rtl.def
index dbf320e..2dcbd8b 100644
--- a/gcc/rtl.def
+++ b/gcc/rtl.def
@@ -823,6 +823,16 @@ DEF_RTL_EXPR(MATCH_CODE, "match_code", "ss", RTX_MATCH)
    appear in a predicate definition or an attribute expression.  */
 DEF_RTL_EXPR(MATCH_TEST, "match_test", "s", RTX_MATCH)
 
+/* Used to match a clobber of flags, or a compare of flags.  The first
+   operand is the rtx of the flags register.  The second operand is an
+   optional rtx expression to match for the compare; if the second 
+   operand is not present, it will be automatically derived from the
+   first set of the parallel in which the match_flags is present.
+   When matching the clobber, the flags register is allowed to have
+   any MODE_CC.  When matching the compare, the mode of the compare is
+   allowed to have any mode compatible with first operand.  */
+DEF_RTL_EXPR(MATCH_FLAGS, "match_flags", "ev", RTX_MATCH)
+
 /* Insn (and related) definitions.  */
 
 /* Definition of the pattern for one kind of instruction.
commit 10b0544386788d1099f52560d517574a8870444f
Author: Richard Henderson <rth@redhat.com>
Date:   Fri Feb 10 15:48:46 2012 -0800

    i386: Use match_flags for any_or patterns

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 6e2c123..fe54e65 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -8125,7 +8125,7 @@
 	(any_or:SWI248
 	 (match_operand:SWI248 1 "nonimmediate_operand" "%0,0")
 	 (match_operand:SWI248 2 "<general_operand>" "<g>,r<i>")))
-   (clobber (reg:CC FLAGS_REG))]
+   (match_flags (reg:CCNO FLAGS_REG))]
   "ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
   "<logic>{<imodesuffix>}\t{%2, %0|%0, %2}"
   [(set_attr "type" "alu")
@@ -8151,7 +8151,7 @@
 	(zero_extend:DI
 	 (any_or:SI (match_operand:SI 1 "nonimmediate_operand" "%0")
 		    (match_operand:SI 2 "x86_64_general_operand" "rme"))))
-   (clobber (reg:CC FLAGS_REG))]
+   (match_flags (reg:CCNO FLAGS_REG))]
   "TARGET_64BIT && ix86_binary_operator_ok (<CODE>, SImode, operands)"
   "<logic>{l}\t{%2, %k0|%k0, %2}"
   [(set_attr "type" "alu")
@@ -8162,7 +8162,7 @@
 	(any_or:DI
 	 (zero_extend:DI (match_operand:SI 1 "register_operand" "%0"))
 	 (match_operand:DI 2 "x86_64_zext_immediate_operand" "Z")))
-   (clobber (reg:CC FLAGS_REG))]
+   (match_flags (reg:CCNO FLAGS_REG))]
   "TARGET_64BIT && ix86_binary_operator_ok (<CODE>, SImode, operands)"
   "<logic>{l}\t{%2, %k0|%k0, %2}"
   [(set_attr "type" "alu")
@@ -8172,65 +8172,8 @@
   [(set (strict_low_part (match_operand:QI 0 "nonimmediate_operand" "+q,m"))
 	(any_or:QI (match_dup 0)
 		   (match_operand:QI 1 "general_operand" "qmn,qn")))
-   (clobber (reg:CC FLAGS_REG))]
-  "(!TARGET_PARTIAL_REG_STALL || optimize_function_for_size_p (cfun))
-   && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
-  "<logic>{b}\t{%1, %0|%0, %1}"
-  [(set_attr "type" "alu1")
-   (set_attr "mode" "QI")])
-
-(define_insn "*<code><mode>_2"
-  [(set (reg FLAGS_REG)
-	(compare (any_or:SWI
-		  (match_operand:SWI 1 "nonimmediate_operand" "%0,0")
-		  (match_operand:SWI 2 "<general_operand>" "<g>,<r><i>"))
-		 (const_int 0)))
-   (set (match_operand:SWI 0 "nonimmediate_operand" "=<r>,<r>m")
-	(any_or:SWI (match_dup 1) (match_dup 2)))]
-  "ix86_match_ccmode (insn, CCNOmode)
-   && ix86_binary_operator_ok (<CODE>, <MODE>mode, operands)"
-  "<logic>{<imodesuffix>}\t{%2, %0|%0, %2}"
-  [(set_attr "type" "alu")
-   (set_attr "mode" "<MODE>")])
-
-;; See comment for addsi_1_zext why we do use nonimmediate_operand
-;; ??? Special case for immediate operand is missing - it is tricky.
-(define_insn "*<code>si_2_zext"
-  [(set (reg FLAGS_REG)
-	(compare (any_or:SI (match_operand:SI 1 "nonimmediate_operand" "%0")
-			    (match_operand:SI 2 "x86_64_general_operand" "rme"))
-		 (const_int 0)))
-   (set (match_operand:DI 0 "register_operand" "=r")
-	(zero_extend:DI (any_or:SI (match_dup 1) (match_dup 2))))]
-  "TARGET_64BIT && ix86_match_ccmode (insn, CCNOmode)
-   && ix86_binary_operator_ok (<CODE>, SImode, operands)"
-  "<logic>{l}\t{%2, %k0|%k0, %2}"
-  [(set_attr "type" "alu")
-   (set_attr "mode" "SI")])
-
-(define_insn "*<code>si_2_zext_imm"
-  [(set (reg FLAGS_REG)
-	(compare (any_or:SI
-		  (match_operand:SI 1 "nonimmediate_operand" "%0")
-		  (match_operand:SI 2 "x86_64_zext_immediate_operand" "Z"))
-		 (const_int 0)))
-   (set (match_operand:DI 0 "register_operand" "=r")
-	(any_or:DI (zero_extend:DI (match_dup 1)) (match_dup 2)))]
-  "TARGET_64BIT && ix86_match_ccmode (insn, CCNOmode)
-   && ix86_binary_operator_ok (<CODE>, SImode, operands)"
-  "<logic>{l}\t{%2, %k0|%k0, %2}"
-  [(set_attr "type" "alu")
-   (set_attr "mode" "SI")])
-
-(define_insn "*<code>qi_2_slp"
-  [(set (reg FLAGS_REG)
-	(compare (any_or:QI (match_operand:QI 0 "nonimmediate_operand" "+q,qm")
-			    (match_operand:QI 1 "general_operand" "qmn,qn"))
-		 (const_int 0)))
-   (set (strict_low_part (match_dup 0))
-	(any_or:QI (match_dup 0) (match_dup 1)))]
+   (match_flags (reg:CCNO FLAGS_REG))]
   "(!TARGET_PARTIAL_REG_STALL || optimize_function_for_size_p (cfun))
-   && ix86_match_ccmode (insn, CCNOmode)
    && !(MEM_P (operands[0]) && MEM_P (operands[1]))"
   "<logic>{b}\t{%1, %0|%0, %1}"
   [(set_attr "type" "alu1")

Comments

Steven Bosscher Feb. 13, 2012, 11:52 p.m. UTC | #1
On Sat, Feb 11, 2012 at 1:12 AM, Richard Henderson <rth@redhat.com> wrote:
> Seeing as how Uros is starting to go down the path of cleaning up the
> flags handling for x86, I thought I'd go ahead and knock up the idea
> that I've been tossing around to help automate the process of building
> patterns that match both clobbering the flags and setting the flags to
> a comparison.
>
> This is far from complete, but it at least shows the direction.
>
> What I know is missing off the top of my head are:
>
>  (0) Documentation in some .texi file; atm there's only what's in rtl.def.
>
>  (1) Generate (clobber (reg flags)) from genemit, should this construct
>     be used in a named insn pattern.
>
>  (2) Can't be usefully used with define_insn_and_split, and no way to tell.
>     This problem should simply be documented in the .texi file as user error.
>
>  (3) Can't be used for x86 add patterns, as the "clobber" version wants the
>     freedom to use "lea" and the "set flags" version cannot.  And there are
>     different sets of constraints if lea may be used or not.
>
>     What would be nice, however, is exposing the targetm.cc_modes_compatible
>     thing in such a way that the x86 add patterns could use that, for the
>     separate insn that does do the set flags.
>
>     Exposing the targetm.cc_modes_compatible thing separately might also
>     clean up some of the evil magic in genrecog.c too.
>
> Comments?

To see if I understand what a cc0 port conversion would look like with
"match_flags", I tried to apply this magic to convert one of the pet
ports, the mighty pdp11.

The pdp11 port doesn't have any define_insn_and_splits, so I didn't
run into the problem you mentioned in (2). What is the problem here? I
suppose it has to do with finding out what the flags setter is after
the split? If so, then couldn't that be resolved with some rules about
how the post-split patterns should be constructed?

Other than that: To convert a port, there is still a lot of work to be
done to define and handle the various CC modes properly (well, not for
the pdp11, because it writes out >1 insn for most define_insns), but
it is great not having to define all the pairs of clobber-flags and
set-flags insns.  At least, I didn't end up rewriting the complete .md
file. It was relatively easy. Less book-keeping involved, etc.

Hope this goes in for GCC 4.8.

Ciao!
Steven
Paolo Bonzini Feb. 14, 2012, 12:26 p.m. UTC | #2
On 02/14/2012 12:52 AM, Steven Bosscher wrote:
> Other than that: To convert a port, there is still a lot of work to be
> done to define and handle the various CC modes properly (well, not for
> the pdp11, because it writes out >1 insn for most define_insns), but
> it is great not having to define all the pairs of clobber-flags and
> set-flags insns.  At least, I didn't end up rewriting the complete .md
> file. It was relatively easy. Less book-keeping involved, etc.

And where's the patch? :)

Paolo
Hans-Peter Nilsson Feb. 23, 2012, 1:16 a.m. UTC | #3
On Fri, 10 Feb 2012, Richard Henderson wrote:
> Seeing as how Uros is starting to go down the path of cleaning up the
> flags handling for x86, I thought I'd go ahead and knock up the idea
> that I've been tossing around to help automate the process of building
> patterns that match both clobbering the flags and setting the flags to
> a comparison.

Sometimes the destination too unless there's overlap, but I
don't know how often that matters.

> This is far from complete, but it at least shows the direction.

Yes, nice!

> What I know is missing off the top of my head are:

>  (2) Can't be usefully used with define_insn_and_split, and no way to tell.
>      This problem should simply be documented in the .texi file as user error.

Not sure I see the problem or the impact of the absence.  Would
it help if there was a way to match_dup the clobber/set?  Maybe
as a match_op_flags, the same as match_flags but with the first
argument being an assigning operand number.  You probably
wouldn't want to use this very often.)

>  (3) Can't be used for x86 add patterns, as the "clobber" version wants the
>      freedom to use "lea" and the "set flags" version cannot.  And there are
>      different sets of constraints if lea may be used or not.

Other targets too, but as it's warty for x86 I hope eventually
this'll see improvement...

> Comments?

Er... very interesting!  (The kind of patch you want to play
around with to have any further insightful comments.)

So I can use match_{operand,whatever} in the second, optional
operand to match_flags?  No, wait, no use to have anything but
a match_dup, hm.

I don't see where it'd make sense to have anything else but the
default, absent argument.  Example?

brgds, H-P
Richard Henderson Feb. 23, 2012, 4:12 p.m. UTC | #4
On 02/22/12 17:16, Hans-Peter Nilsson wrote:
>> What I know is missing off the top of my head are:
> 
>>  (2) Can't be usefully used with define_insn_and_split, and no way to tell.
>>      This problem should simply be documented in the .texi file as user error.
> 
> Not sure I see the problem or the impact of the absence.  Would
> it help if there was a way to match_dup the clobber/set?  Maybe
> as a match_op_flags, the same as match_flags but with the first
> argument being an assigning operand number.  You probably
> wouldn't want to use this very often.)

Yes, one could probably have some use of it with a split, if the
match_flags is assigned an operand number.  You'd have to be
very careful about preserving the contents of the compare if you
adjust the other instruction data at all...

I've also thought of using the operand number as a quick way to
test whether the flags are actually live.  I.e.

  if (GET_CODE (operands[n]) == CLOBBER)

within the split or the C output template.


r~
Steven Bosscher May 17, 2012, 5:59 p.m. UTC | #5
On Sat, Feb 11, 2012 at 1:12 AM, Richard Henderson <rth@redhat.com> wrote:
> Seeing as how Uros is starting to go down the path of cleaning up the
> flags handling for x86, I thought I'd go ahead and knock up the idea
> that I've been tossing around to help automate the process of building
> patterns that match both clobbering the flags and setting the flags to
> a comparison.
>
> This is far from complete, but it at least shows the direction.
>
> What I know is missing off the top of my head are:
>
>  (0) Documentation in some .texi file; atm there's only what's in rtl.def.
>
>  (1) Generate (clobber (reg flags)) from genemit, should this construct
>     be used in a named insn pattern.
>
>  (2) Can't be usefully used with define_insn_and_split, and no way to tell.
>     This problem should simply be documented in the .texi file as user error.
>
>  (3) Can't be used for x86 add patterns, as the "clobber" version wants the
>     freedom to use "lea" and the "set flags" version cannot.  And there are
>     different sets of constraints if lea may be used or not.
>
>     What would be nice, however, is exposing the targetm.cc_modes_compatible
>     thing in such a way that the x86 add patterns could use that, for the
>     separate insn that does do the set flags.
>
>     Exposing the targetm.cc_modes_compatible thing separately might also
>     clean up some of the evil magic in genrecog.c too.
>
> Comments?


Hello Richard,

Are you still working on this for GCC 4.8?

Ciao!
Steven
Richard Henderson May 17, 2012, 6:04 p.m. UTC | #6
On 05/17/12 10:59, Steven Bosscher wrote:
> Are you still working on this for GCC 4.8?

Not actively.


r~
diff mbox

Patch

diff --git a/gcc/doc/rtl.texi b/gcc/doc/rtl.texi
index 7e9e535..b0261e3 100644
--- a/gcc/doc/rtl.texi
+++ b/gcc/doc/rtl.texi
@@ -250,6 +250,12 @@  core, @samp{S} is equivalent to @samp{s}, but when the object is read,
 from an @samp{md} file, the string value of this operand may be omitted.
 An omitted string is taken to be the null string.
 
+@item v
+@samp{v} indicates an expression which is optional.  In the RTL objects
+in core, @samp{v} is equivalent to @samp{e}, but when the object is
+read from an @samp{md} file, the expression for this operand may be
+omitted.
+
 @item V
 @samp{V} indicates a vector which is optional.  In the RTL objects in
 core, @samp{V} is equivalent to @samp{E}, but when the object is read
diff --git a/gcc/gengenrtl.c b/gcc/gengenrtl.c
index 67688ac..99765a7 100644
--- a/gcc/gengenrtl.c
+++ b/gcc/gengenrtl.c
@@ -58,10 +58,10 @@  type_from_format (int c)
     case 's':
       return "const char *";
 
-    case 'e':  case 'u':
+    case 'e': case 'u': case 'v':
       return "rtx ";
 
-    case 'E':
+    case 'E': case 'V':
       return "rtvec ";
     case 't':
       return "union tree_node *";  /* tree - typedef not available */
@@ -88,10 +88,10 @@  accessor_from_format (int c)
     case 's':
       return "XSTR";
 
-    case 'e':  case 'u':
+    case 'e': case 'u': case 'v':
       return "XEXP";
 
-    case 'E':
+    case 'E': case 'V':
       return "XVEC";
 
     case 't':
diff --git a/gcc/print-rtl.c b/gcc/print-rtl.c
index 9bd8621..a83cb4f 100644
--- a/gcc/print-rtl.c
+++ b/gcc/print-rtl.c
@@ -361,7 +361,7 @@  print_rtx (const_rtx in_rtx)
 	  }
 	break;
 
-      case 'e':
+      case 'e': case 'v':
       do_e:
 	indent += 2;
 	if (i == 7 && INSN_P (in_rtx))
diff --git a/gcc/read-rtl.c b/gcc/read-rtl.c
index 1402c54..a5d4ccb 100644
--- a/gcc/read-rtl.c
+++ b/gcc/read-rtl.c
@@ -363,6 +363,7 @@  apply_iterator_to_rtx (rtx original, struct mapping *iterator, int value,
 	XSTR (x, i) = apply_iterator_to_string (XSTR (x, i), iterator, value);
 	break;
 
+      case 'v':
       case 'e':
 	XEXP (x, i) = apply_iterator_to_rtx (XEXP (x, i), iterator, value,
 					     mode_maps, unknown_mode_attr);
@@ -406,6 +407,7 @@  uses_iterator_p (rtx x, struct mapping *iterator)
   for (i = 0; format_ptr[i] != 0; i++)
     switch (format_ptr[i])
       {
+      case 'v':
       case 'e':
 	if (uses_iterator_p (XEXP (x, i), iterator))
 	  return true;
@@ -929,6 +931,18 @@  read_rtx_code (const char *code_name, struct map_value **mode_maps)
       case '0':
 	break;
 
+      case 'v':
+	/* 'v' is an optional expression: if a closeparen follows,
+	   just store NULL for this element.  */
+	c = read_skip_spaces ();
+	unread_char (c);
+	if (c == ')')
+	  {
+	    XEXP (return_rtx, i) = NULL;
+	    break;
+	  }
+	/* FALLTHRU */
+
       case 'e':
       case 'u':
 	XEXP (return_rtx, i) = read_nested_rtx (mode_maps);
@@ -944,7 +958,7 @@  read_rtx_code (const char *code_name, struct map_value **mode_maps)
 	    XVEC (return_rtx, i) = 0;
 	    break;
 	  }
-	/* Now process the vector.  */
+	/* FALLTHRU */
 
       case 'E':
 	{