Patchwork Allow match_test to be used for .md attribute tests

login
register
mail settings
Submitter Richard Sandiford
Date Aug. 13, 2011, 10:15 a.m.
Message ID <87pqk939dn.fsf@firetop.home>
Download mbox | patch
Permalink /patch/109929/
State New
Headers show

Comments

Richard Sandiford - Aug. 13, 2011, 10:15 a.m.
Like the recent MEM_ATTRS changes, this patch is supposed to help the
transition to const_ints with modes.  However, like those changes,
I think it makes sense on its own.

At the moment, the canonical way of testing a C condition in an
.md attribute is to use:

    (ne (symbol_ref "TEST") (const_int 0))

The patch below allows the predicate construct:

    (match_test "TEST")

to be used instead, which is both shorter and IMO easier to read.
The other advantage for me is that it gets rid of some "fake" const_ints.

Looking at genattrtab.c, I'm not sure whether the use of symbol_ref
in these kinds of attribute test was a deliberate feature, or whether
it's something that worked by accident (on the back of the support for
symbol_ref attribute values, which are definitely a deliberate feature).
There's code to create unique rtxes for comparisons of two symbol_ref
attribute values:

    case LE:  case LT:  case GT:  case GE:
    case LEU: case LTU: case GTU: case GEU:
    case NE:  case EQ:
      if (GET_CODE (XEXP (exp, 0)) == SYMBOL_REF
	  && GET_CODE (XEXP (exp, 1)) == SYMBOL_REF)
	exp = attr_rtx (GET_CODE (exp),
			attr_rtx (SYMBOL_REF, XSTR (XEXP (exp, 0), 0)),
			attr_rtx (SYMBOL_REF, XSTR (XEXP (exp, 1), 0)));

but no corresponding code for (foo (symbol_ref ...) (const_int 0)).
This means that two separate occurences of:

    (ne (symbol_ref "TEST") (const_int 0))

in the .md file will appear to genattrtab as two separate conditions
whose relationship is unknown.

As well as the usual bootstrap and regression testing, I tried applying:
Hans-Peter Nilsson - Aug. 13, 2011, 10:31 a.m.
On Sat, 13 Aug 2011, Richard Sandiford wrote:
> If the patch is OK, I'd like to make corresponding changes to each port's
> .md files.  Are they simple enough to count as obvious, or should I get
> persmission for each one?

Preapproved for CRIS and MMIX.

brgds, H-P

Patch

Index: gcc/genattrtab.c
===================================================================
--- gcc/genattrtab.c	2011-08-13 09:11:33.000000000 +0100
+++ gcc/genattrtab.c	2011-08-13 09:43:19.000000000 +0100
@@ -857,6 +857,25 @@  check_attr_test (rtx exp, int is_const,
 	exp = attr_rtx (GET_CODE (exp),
 			attr_rtx (SYMBOL_REF, XSTR (XEXP (exp, 0), 0)),
 			attr_rtx (SYMBOL_REF, XSTR (XEXP (exp, 1), 0)));
+      if (GET_CODE (exp) == NE
+	  && GET_CODE (XEXP (exp, 0)) == SYMBOL_REF
+	  && CONST_INT_P (XEXP (exp, 1))
+	  && XWINT (XEXP (exp, 1), 0) == 0)
+	exp = attr_rtx (NE,
+			attr_rtx (SYMBOL_REF, XSTR (XEXP (exp, 0), 0)),
+			false_rtx);
+      if (GET_CODE (exp) == EQ
+	  && GET_CODE (XEXP (exp, 0)) == SYMBOL_REF
+	  && CONST_INT_P (XEXP (exp, 1))
+	  && XWINT (XEXP (exp, 1), 0) == 0)
+	{
+	  exp = attr_rtx (NE,
+			  attr_rtx (SYMBOL_REF, XSTR (XEXP (exp, 0), 0)),
+			  false_rtx);
+	  ATTR_IND_SIMPLIFIED_P (exp) = 1;
+	  exp = attr_rtx (NOT, exp);
+	  break;
+	}
       /* These cases can't be simplified.  */
       ATTR_IND_SIMPLIFIED_P (exp) = 1;
       break;

to an otherwise unpatched compiler.  This simplified the insn-attrtab.c
output, because genattrtab was able to merge some case statements with
identical bodies.  I used this as the "old" version of insn-attrtab.c.

I then applied the patch at the end of this message, the i386 patch that
I'm about to post, and a MIPS patch that I'll post if the patch below
is OK.  I also applied:

Index: gcc/genattrtab.c
===================================================================
--- gcc/genattrtab.c	2011-08-13 09:43:46.000000000 +0100
+++ gcc/genattrtab.c	2011-08-13 09:44:02.000000000 +0100
@@ -3598,9 +3598,9 @@  write_test_expr (rtx exp, unsigned int a
       break;
 
     case MATCH_TEST:
+      printf ("(");
       print_c_condition (XSTR (exp, 0));
-      if (flags & FLG_BITWISE)
-	printf (" != 0");
+      printf (") != (0)");
       break;
 
     /* A random C expression.  */

in order to make the new output look more like the original.  I used the
insn-attrtab.c generated by this new genattrtab as the "new" version.
After removing #line directives, the old and new versions were the same,
apart from cases where I'd split a single symbol_ref condition into two
match_tests.

Bootstrapped & regression-tested on x86_64-linux-gnu with the next patch.
Also regression-tested on mips64-linux-gnu with the corresponding .md patch
to config/mips.  (I'll post and commit that if these changes are OK.
It'd just be noise otherwise.)  OK to install?

If the patch is OK, I'd like to make corresponding changes to each port's
.md files.  Are they simple enough to count as obvious, or should I get
persmission for each one?

Richard


gcc/
	* doc/md.texi: Describe the use of match_tests in attribute tests.
	* rtl.def (MATCH_TEST): Update commentary.
	* genattrtab.c (attr_copy_rtx, check_attr_test, clear_struct_flag)
	(write_test_expr, walk_attr_value): Handle MATCH_TEST.

Index: gcc/doc/md.texi
===================================================================
--- gcc/doc/md.texi	2011-08-13 09:42:09.000000000 +0100
+++ gcc/doc/md.texi	2011-08-13 09:43:46.000000000 +0100
@@ -7031,6 +7031,30 @@  string).
 
 The @var{constraints} operand is ignored and should be the null string.
 
+@cindex @code{match_test} and attributes
+@item (match_test @var{c-expr})
+The test is true if C expression @var{c-expr} is true.  In non-constant
+attributes, @var{c-expr} has access to the following variables:
+
+@table @var
+@item insn
+The rtl instruction under test.
+@item which_alternative
+The @code{define_insn} alternative that @var{insn} matches.
+@xref{Output Statement}.
+@item operands
+An array of @var{insn}'s rtl operands.
+@end table
+
+@var{c-expr} behaves like the condition in a C @code{if} statement,
+so there is no need to explicitly convert the expression into a boolean
+0 or 1 value.  For example, the following two tests are equivalent:
+
+@smallexample
+(match_test "x & 2")
+(match_test "(x & 2) != 0")
+@end smallexample
+
 @cindex @code{le} and attributes
 @cindex @code{leu} and attributes
 @cindex @code{lt} and attributes
Index: gcc/rtl.def
===================================================================
--- gcc/rtl.def	2011-08-13 09:42:09.000000000 +0100
+++ gcc/rtl.def	2011-08-13 09:43:46.000000000 +0100
@@ -813,9 +813,8 @@  DEF_RTL_EXPR(MATCH_PAR_DUP, "match_par_d
    the result of the one before it.  */
 DEF_RTL_EXPR(MATCH_CODE, "match_code", "ss", RTX_MATCH)
 
-/* Appears only in define_predicate/define_special_predicate
-    expressions.  The argument is a C expression to be injected at this
-    point in the predicate formula.  */
+/* Used to inject a C conditional expression into an .md file.  It can
+   appear in a predicate definition or an attribute expression.  */
 DEF_RTL_EXPR(MATCH_TEST, "match_test", "s", RTX_MATCH)
 
 /* Insn (and related) definitions.  */
Index: gcc/genattrtab.c
===================================================================
--- gcc/genattrtab.c	2011-08-13 09:43:19.000000000 +0100
+++ gcc/genattrtab.c	2011-08-13 09:46:12.000000000 +0100
@@ -658,6 +658,7 @@  attr_copy_rtx (rtx orig)
     case CONST_DOUBLE:
     case CONST_VECTOR:
     case SYMBOL_REF:
+    case MATCH_TEST:
     case CODE_LABEL:
     case PC:
     case CC0:
@@ -841,6 +842,11 @@  check_attr_test (rtx exp, int is_const,
       XEXP (exp, 0) = check_attr_test (XEXP (exp, 0), is_const, lineno);
       break;
 
+    case MATCH_TEST:
+      exp = attr_rtx (MATCH_TEST, XSTR (exp, 0));
+      ATTR_IND_SIMPLIFIED_P (exp) = 1;
+      break;
+
     case MATCH_OPERAND:
       if (is_const)
 	fatal ("RTL operator \"%s\" not valid in constant attribute test",
@@ -2926,6 +2932,7 @@  clear_struct_flag (rtx x)
     case CONST_INT:
     case CONST_DOUBLE:
     case CONST_VECTOR:
+    case MATCH_TEST:
     case SYMBOL_REF:
     case CODE_LABEL:
     case PC:
@@ -3590,6 +3597,12 @@  write_test_expr (rtx exp, unsigned int a
       printf (HOST_WIDE_INT_PRINT_DEC, XWINT (exp, 0));
       break;
 
+    case MATCH_TEST:
+      print_c_condition (XSTR (exp, 0));
+      if (flags & FLG_BITWISE)
+	printf (" != 0");
+      break;
+
     /* A random C expression.  */
     case SYMBOL_REF:
       print_c_condition (XSTR (exp, 0));
@@ -3784,6 +3797,7 @@  walk_attr_value (rtx exp)
       must_extract = 1;
       return;
 
+    case MATCH_TEST:
     case EQ_ATTR_ALT:
       must_extract = must_constrain = 1;
       break;