[PATCH v2] Change EQ_ATTR_ALT to support up to 64 alternatives

Ilya Leoshkevich iii@linux.ibm.com
Thu Sep 20 09:06:00 GMT 2018


Bootstrapped and regtested on x86_64-redhat-linux and
s390x-redhat-linux.

Changes since v1:

* Use alternative_mask and HOST_WIDE_INT in attr_alt_intersection,
  attr_alt_union and attr_alt_complement.

On S/390 there is a need to support more than 32 instruction
alternatives per define_insn.  Currently this is not explicitly
prohibited or unsupported: MAX_RECOG_ALTERNATIVES is equal 35, and,
futhermore, the related code uses uint64_t for bitmaps in most places.

However, genattrtab contains the logic to convert (eq_attr "attribute"
"value") RTXs to (eq_attr_alt bitmap) RTXs, where bitmap contains
alternatives, whose "attribute" has the corresponding "value".
Unfortunately, bitmap is only 32 bits.

When adding the 33rd alternative, this led to (eq_attr "type" "larl")
becoming (eq_attr_alt -1050625 1), where -1050625 == 0xffeff7ff.  The
cleared bits 12, 21 and 32 correspond to two existing and one newly
added insn of type "larl".  compute_alternative_mask sign extended this
to 0xffffffffffeff7ff, which contained non-existent alternatives, and
this made simplify_test_exp fail with "invalid alternative specified".

I'm not sure why it didn't fail the same way before, since the top bit,
which led to sign extension, should have been set even with 32
alternatives.  Maybe simplify_test_exp was not called for "type"
attribute for some reason?

This patch widens EQ_ATTR_ALT bitmap to 64 bits, making it possible to
gracefully handle up to 64 alternatives.  It eliminates the problem with
the 33rd alternative on S/390.

gcc/ChangeLog:

2018-09-18  Ilya Leoshkevich  <iii@linux.ibm.com>

	* genattrtab.c (mk_attr_alt): Use alternative_mask.
	(attr_rtx_1): Adjust caching to match the new EQ_ATTR_ALT field
        types.
	(check_attr_test): Use alternative_mask.
	(get_attr_value): Likewise.
	(compute_alternative_mask): Use alternative_mask and XWINT.
	(make_alternative_compare): Use alternative_mask.
	(attr_alt_subset_p): Use XWINT.
	(attr_alt_subset_of_compl_p): Likewise.
	(attr_alt_intersection): Use alternative_mask and XWINT.
	(attr_alt_union): Likewise.
	(attr_alt_complement): Use HOST_WIDE_INT and XWINT.
        (mk_attr_alt): Use alternative_mask and HOST_WIDE_INT.
	(simplify_test_exp): Use alternative_mask and XWINT.
	(write_test_expr): Use alternative_mask and XWINT, adjust bit
        number calculation to support 64 bits.  Generate code that
        checks 64-bit masks.
	(main): Use alternative_mask.
	* rtl.def (EQ_ATTR_ALT): Change field types from ii to ww.
---
 gcc/genattrtab.c | 132 ++++++++++++++++++++++++++---------------------
 gcc/recog.h      |   2 +-
 gcc/rtl.def      |   2 +-
 3 files changed, 74 insertions(+), 62 deletions(-)

diff --git a/gcc/genattrtab.c b/gcc/genattrtab.c
index f9b0bc94b0f..d5cdbf5be23 100644
--- a/gcc/genattrtab.c
+++ b/gcc/genattrtab.c
@@ -228,7 +228,9 @@ static int *insn_n_alternatives;
 /* Stores, for each insn code, a bitmap that has bits on for each possible
    alternative.  */
 
-static uint64_t *insn_alternatives;
+/* Keep this in sync with recog.h.  */
+typedef uint64_t alternative_mask;
+static alternative_mask *insn_alternatives;
 
 /* Used to simplify expressions.  */
 
@@ -256,7 +258,7 @@ static char *attr_printf           (unsigned int, const char *, ...)
   ATTRIBUTE_PRINTF_2;
 static rtx make_numeric_value      (int);
 static struct attr_desc *find_attr (const char **, int);
-static rtx mk_attr_alt             (uint64_t);
+static rtx mk_attr_alt             (alternative_mask);
 static char *next_comma_elt	   (const char **);
 static rtx insert_right_side	   (enum rtx_code, rtx, rtx, int, int);
 static rtx copy_boolean		   (rtx);
@@ -494,26 +496,26 @@ attr_rtx_1 (enum rtx_code code, va_list p)
 	}
     }
   else if (GET_RTX_LENGTH (code) == 2
-	   && GET_RTX_FORMAT (code)[0] == 'i'
-	   && GET_RTX_FORMAT (code)[1] == 'i')
+	   && GET_RTX_FORMAT (code)[0] == 'w'
+	   && GET_RTX_FORMAT (code)[1] == 'w')
     {
-      int  arg0 = va_arg (p, int);
-      int  arg1 = va_arg (p, int);
+      HOST_WIDE_INT arg0 = va_arg (p, HOST_WIDE_INT);
+      HOST_WIDE_INT arg1 = va_arg (p, HOST_WIDE_INT);
 
       hashcode = ((HOST_WIDE_INT) code + RTL_HASH (arg0) + RTL_HASH (arg1));
       for (h = attr_hash_table[hashcode % RTL_HASH_SIZE]; h; h = h->next)
 	if (h->hashcode == hashcode
 	    && GET_CODE (h->u.rtl) == code
-	    && XINT (h->u.rtl, 0) == arg0
-	    && XINT (h->u.rtl, 1) == arg1)
+	    && XWINT (h->u.rtl, 0) == arg0
+	    && XWINT (h->u.rtl, 1) == arg1)
 	  return h->u.rtl;
 
       if (h == 0)
 	{
 	  rtl_obstack = hash_obstack;
 	  rt_val = rtx_alloc (code);
-	  XINT (rt_val, 0) = arg0;
-	  XINT (rt_val, 1) = arg1;
+	  XWINT (rt_val, 0) = arg0;
+	  XWINT (rt_val, 1) = arg1;
 	}
     }
   else if (code == CONST_INT)
@@ -703,7 +705,8 @@ check_attr_test (file_location loc, rtx exp, attr_desc *attr)
 	  if (attr2 == NULL)
 	    {
 	      if (! strcmp (XSTR (exp, 0), "alternative"))
-		return mk_attr_alt (((uint64_t) 1) << atoi (XSTR (exp, 1)));
+		return mk_attr_alt (((alternative_mask) 1)
+				    << atoi (XSTR (exp, 1)));
 	      else
 		fatal_at (loc, "unknown attribute `%s' in definition of"
 			  " attribute `%s'", XSTR (exp, 0), attr->name);
@@ -750,7 +753,7 @@ check_attr_test (file_location loc, rtx exp, attr_desc *attr)
 
 	      name_ptr = XSTR (exp, 1);
 	      while ((p = next_comma_elt (&name_ptr)) != NULL)
-		set |= ((uint64_t) 1) << atoi (p);
+		set |= ((alternative_mask) 1) << atoi (p);
 
 	      return mk_attr_alt (set);
 	    }
@@ -1224,7 +1227,7 @@ get_attr_value (file_location loc, rtx value, struct attr_desc *attr,
 		int insn_code)
 {
   struct attr_value *av;
-  uint64_t num_alt = 0;
+  alternative_mask num_alt = 0;
 
   value = make_canonical (loc, attr, value);
   if (compares_alternatives_p (value))
@@ -1868,7 +1871,7 @@ insert_right_side (enum rtx_code code, rtx exp, rtx term, int insn_code, int ins
    This routine is passed an expression and either AND or IOR.  It returns a
    bitmask indicating which alternatives are mentioned within EXP.  */
 
-static uint64_t
+static alternative_mask
 compute_alternative_mask (rtx exp, enum rtx_code code)
 {
   const char *string;
@@ -1887,11 +1890,11 @@ compute_alternative_mask (rtx exp, enum rtx_code code)
 
   else if (GET_CODE (exp) == EQ_ATTR_ALT)
     {
-      if (code == AND && XINT (exp, 1))
-	return XINT (exp, 0);
+      if (code == AND && XWINT (exp, 1))
+	return XWINT (exp, 0);
 
-      if (code == IOR && !XINT (exp, 1))
-	return XINT (exp, 0);
+      if (code == IOR && !XWINT (exp, 1))
+	return XWINT (exp, 0);
 
       return 0;
     }
@@ -1899,15 +1902,15 @@ compute_alternative_mask (rtx exp, enum rtx_code code)
     return 0;
 
   if (string[1] == 0)
-    return ((uint64_t) 1) << (string[0] - '0');
-  return ((uint64_t) 1) << atoi (string);
+    return ((alternative_mask) 1) << (string[0] - '0');
+  return ((alternative_mask) 1) << atoi (string);
 }
 
 /* Given I, a single-bit mask, return RTX to compare the `alternative'
    attribute with the value represented by that bit.  */
 
 static rtx
-make_alternative_compare (uint64_t mask)
+make_alternative_compare (alternative_mask mask)
 {
   return mk_attr_alt (mask);
 }
@@ -2286,19 +2289,19 @@ simplify_test_exp_in_temp (rtx exp, int insn_code, int insn_index)
 static bool
 attr_alt_subset_p (rtx s1, rtx s2)
 {
-  switch ((XINT (s1, 1) << 1) | XINT (s2, 1))
+  switch ((XWINT (s1, 1) << 1) | XWINT (s2, 1))
     {
     case (0 << 1) | 0:
-      return !(XINT (s1, 0) &~ XINT (s2, 0));
+      return !(XWINT (s1, 0) &~ XWINT (s2, 0));
 
     case (0 << 1) | 1:
-      return !(XINT (s1, 0) & XINT (s2, 0));
+      return !(XWINT (s1, 0) & XWINT (s2, 0));
 
     case (1 << 1) | 0:
       return false;
 
     case (1 << 1) | 1:
-      return !(XINT (s2, 0) &~ XINT (s1, 0));
+      return !(XWINT (s2, 0) &~ XWINT (s1, 0));
 
     default:
       gcc_unreachable ();
@@ -2310,16 +2313,16 @@ attr_alt_subset_p (rtx s1, rtx s2)
 static bool
 attr_alt_subset_of_compl_p (rtx s1, rtx s2)
 {
-  switch ((XINT (s1, 1) << 1) | XINT (s2, 1))
+  switch ((XWINT (s1, 1) << 1) | XWINT (s2, 1))
     {
     case (0 << 1) | 0:
-      return !(XINT (s1, 0) & XINT (s2, 0));
+      return !(XWINT (s1, 0) & XWINT (s2, 0));
 
     case (0 << 1) | 1:
-      return !(XINT (s1, 0) & ~XINT (s2, 0));
+      return !(XWINT (s1, 0) & ~XWINT (s2, 0));
 
     case (1 << 1) | 0:
-      return !(XINT (s2, 0) &~ XINT (s1, 0));
+      return !(XWINT (s2, 0) &~ XWINT (s1, 0));
 
     case (1 << 1) | 1:
       return false;
@@ -2334,27 +2337,27 @@ attr_alt_subset_of_compl_p (rtx s1, rtx s2)
 static rtx
 attr_alt_intersection (rtx s1, rtx s2)
 {
-  int result;
+  alternative_mask result;
 
-  switch ((XINT (s1, 1) << 1) | XINT (s2, 1))
+  switch ((XWINT (s1, 1) << 1) | XWINT (s2, 1))
     {
     case (0 << 1) | 0:
-      result = XINT (s1, 0) & XINT (s2, 0);
+      result = XWINT (s1, 0) & XWINT (s2, 0);
       break;
     case (0 << 1) | 1:
-      result = XINT (s1, 0) & ~XINT (s2, 0);
+      result = XWINT (s1, 0) & ~XWINT (s2, 0);
       break;
     case (1 << 1) | 0:
-      result = XINT (s2, 0) & ~XINT (s1, 0);
+      result = XWINT (s2, 0) & ~XWINT (s1, 0);
       break;
     case (1 << 1) | 1:
-      result = XINT (s1, 0) | XINT (s2, 0);
+      result = XWINT (s1, 0) | XWINT (s2, 0);
       break;
     default:
       gcc_unreachable ();
     }
 
-  return attr_rtx (EQ_ATTR_ALT, result, XINT (s1, 1) & XINT (s2, 1));
+  return attr_rtx (EQ_ATTR_ALT, result, XWINT (s1, 1) & XWINT (s2, 1));
 }
 
 /* Return EQ_ATTR_ALT expression representing union of S1 and S2.  */
@@ -2362,27 +2365,27 @@ attr_alt_intersection (rtx s1, rtx s2)
 static rtx
 attr_alt_union (rtx s1, rtx s2)
 {
-  int result;
+  alternative_mask result;
 
-  switch ((XINT (s1, 1) << 1) | XINT (s2, 1))
+  switch ((XWINT (s1, 1) << 1) | XWINT (s2, 1))
     {
     case (0 << 1) | 0:
-      result = XINT (s1, 0) | XINT (s2, 0);
+      result = XWINT (s1, 0) | XWINT (s2, 0);
       break;
     case (0 << 1) | 1:
-      result = XINT (s2, 0) & ~XINT (s1, 0);
+      result = XWINT (s2, 0) & ~XWINT (s1, 0);
       break;
     case (1 << 1) | 0:
-      result = XINT (s1, 0) & ~XINT (s2, 0);
+      result = XWINT (s1, 0) & ~XWINT (s2, 0);
       break;
     case (1 << 1) | 1:
-      result = XINT (s1, 0) & XINT (s2, 0);
+      result = XWINT (s1, 0) & XWINT (s2, 0);
       break;
     default:
       gcc_unreachable ();
     }
 
-  return attr_rtx (EQ_ATTR_ALT, result, XINT (s1, 1) | XINT (s2, 1));
+  return attr_rtx (EQ_ATTR_ALT, result, XWINT (s1, 1) | XWINT (s2, 1));
 }
 
 /* Return EQ_ATTR_ALT expression representing complement of S.  */
@@ -2390,16 +2393,17 @@ attr_alt_union (rtx s1, rtx s2)
 static rtx
 attr_alt_complement (rtx s)
 {
-  return attr_rtx (EQ_ATTR_ALT, XINT (s, 0), 1 - XINT (s, 1));
+  return attr_rtx (EQ_ATTR_ALT, XWINT (s, 0),
+                   ((HOST_WIDE_INT) 1) - XWINT (s, 1));
 }
 
 /* Return EQ_ATTR_ALT expression representing set containing elements set
    in E.  */
 
 static rtx
-mk_attr_alt (uint64_t e)
+mk_attr_alt (alternative_mask e)
 {
-  return attr_rtx (EQ_ATTR_ALT, (int)e, 0);
+  return attr_rtx (EQ_ATTR_ALT, (HOST_WIDE_INT) e, (HOST_WIDE_INT) 0);
 }
 
 /* Given an expression, see if it can be simplified for a particular insn
@@ -2419,7 +2423,7 @@ simplify_test_exp (rtx exp, int insn_code, int insn_index)
   struct attr_value *av;
   struct insn_ent *ie;
   struct attr_value_list *iv;
-  uint64_t i;
+  alternative_mask i;
   rtx newexp = exp;
   bool left_alt, right_alt;
 
@@ -2484,14 +2488,14 @@ simplify_test_exp (rtx exp, int insn_code, int insn_index)
 		    && XSTR (XEXP (left, 0), 0) == alternative_name);
       else
 	left_alt = (GET_CODE (left) == EQ_ATTR_ALT
-		    && XINT (left, 1));
+		    && XWINT (left, 1));
 
       if (GET_CODE (right) == NOT)
 	right_alt = (GET_CODE (XEXP (right, 0)) == EQ_ATTR
 		     && XSTR (XEXP (right, 0), 0) == alternative_name);
       else
 	right_alt = (GET_CODE (right) == EQ_ATTR_ALT
-		     && XINT (right, 1));
+		     && XWINT (right, 1));
 
       if (insn_code >= 0
 	  && (GET_CODE (left) == AND
@@ -2602,12 +2606,12 @@ simplify_test_exp (rtx exp, int insn_code, int insn_index)
       else if (insn_code >= 0
 	       && (GET_CODE (left) == IOR
 		   || (GET_CODE (left) == EQ_ATTR_ALT
-		       && !XINT (left, 1))
+		       && !XWINT (left, 1))
 		   || (GET_CODE (left) == EQ_ATTR
 		       && XSTR (left, 0) == alternative_name)
 		   || GET_CODE (right) == IOR
 		   || (GET_CODE (right) == EQ_ATTR_ALT
-		       && !XINT (right, 1))
+		       && !XWINT (right, 1))
 		   || (GET_CODE (right) == EQ_ATTR
 		       && XSTR (right, 0) == alternative_name)))
 	{
@@ -2688,14 +2692,15 @@ simplify_test_exp (rtx exp, int insn_code, int insn_index)
       break;
 
     case EQ_ATTR_ALT:
-      if (!XINT (exp, 0))
-	return XINT (exp, 1) ? true_rtx : false_rtx;
+      if (!XWINT (exp, 0))
+	return XWINT (exp, 1) ? true_rtx : false_rtx;
       break;
 
     case EQ_ATTR:
       if (XSTR (exp, 0) == alternative_name)
 	{
-	  newexp = mk_attr_alt (((uint64_t) 1) << atoi (XSTR (exp, 1)));
+	  newexp = mk_attr_alt (((alternative_mask) 1)
+				<< atoi (XSTR (exp, 1)));
 	  break;
 	}
 
@@ -3568,7 +3573,8 @@ write_test_expr (FILE *outf, rtx exp, unsigned int attrs_cached, int flags,
 
     case EQ_ATTR_ALT:
 	{
-	  int set = XINT (exp, 0), bit = 0;
+	  alternative_mask set = XWINT (exp, 0);
+	  int bit = 0;
 
 	  if (flags & FLG_BITWISE)
 	    fatal ("EQ_ATTR_ALT not valid inside comparison");
@@ -3578,6 +3584,11 @@ write_test_expr (FILE *outf, rtx exp, unsigned int attrs_cached, int flags,
 
 	  if (!(set & (set - 1)))
 	    {
+	      if (!(set & 0xffffffff))
+		{
+		  bit += 32;
+		  set >>= 32;
+		}
 	      if (!(set & 0xffff))
 		{
 		  bit += 16;
@@ -3602,12 +3613,13 @@ write_test_expr (FILE *outf, rtx exp, unsigned int attrs_cached, int flags,
 		bit++;
 
 	      fprintf (outf, "which_alternative %s= %d",
-		       XINT (exp, 1) ? "!" : "=", bit);
+		       XWINT (exp, 1) ? "!" : "=", bit);
 	    }
 	  else
 	    {
-	      fprintf (outf, "%s((1 << which_alternative) & %#x)",
-		       XINT (exp, 1) ? "!" : "", set);
+	      fprintf (outf, "%s((1ULL << which_alternative) & %#" PRIx64
+			     "ULL)",
+		       XWINT (exp, 1) ? "!" : "", set);
 	    }
 	}
       break;
@@ -5220,11 +5232,11 @@ main (int argc, const char **argv)
 
   /* Make `insn_alternatives'.  */
   int num_insn_codes = get_num_insn_codes ();
-  insn_alternatives = oballocvec (uint64_t, num_insn_codes);
+  insn_alternatives = oballocvec (alternative_mask, num_insn_codes);
   for (id = defs; id; id = id->next)
     if (id->insn_code >= 0)
       insn_alternatives[id->insn_code]
-	= (((uint64_t) 1) << id->num_alternatives) - 1;
+	= (((alternative_mask) 1) << id->num_alternatives) - 1;
 
   /* Make `insn_n_alternatives'.  */
   insn_n_alternatives = oballocvec (int, num_insn_codes);
diff --git a/gcc/recog.h b/gcc/recog.h
index eca62803458..3d417ea23ac 100644
--- a/gcc/recog.h
+++ b/gcc/recog.h
@@ -24,7 +24,7 @@ along with GCC; see the file COPYING3.  If not see
    a type that has at least MAX_RECOG_ALTERNATIVES + 1 bits, with the extra
    bit giving an invalid value that can be used to mean "uninitialized".  */
 #define MAX_RECOG_ALTERNATIVES 35
-typedef uint64_t alternative_mask;
+typedef uint64_t alternative_mask;  /* Keep in sync with genattrtab.c.  */
 
 /* A mask of all alternatives.  */
 #define ALL_ALTERNATIVES ((alternative_mask) -1)
diff --git a/gcc/rtl.def b/gcc/rtl.def
index 0ed27505545..b4282ab7ffd 100644
--- a/gcc/rtl.def
+++ b/gcc/rtl.def
@@ -1310,7 +1310,7 @@ DEF_RTL_EXPR(EQ_ATTR, "eq_attr", "ss", RTX_EXTRA)
 
 /* A special case of the above representing a set of alternatives.  The first
    operand is bitmap of the set, the second one is the default value.  */
-DEF_RTL_EXPR(EQ_ATTR_ALT, "eq_attr_alt", "ii", RTX_EXTRA)
+DEF_RTL_EXPR(EQ_ATTR_ALT, "eq_attr_alt", "ww", RTX_EXTRA)
 
 /* A conditional expression which is true if the specified flag is
    true for the insn being scheduled in reorg.
-- 
2.19.0



More information about the Gcc-patches mailing list