Re: [PATCH] genattrtab bit-rot, and if_then_else in values

[Richard Sandiford <richard.sandiford@arm.com>]

Alan Modra <amodra@gmail.com> writes:
> On Thu, Jan 03, 2019 at 07:03:59PM +0000, Richard Sandiford wrote:
>> Richard Sandiford <richard.sandiford@arm.com> writes:
>> > This still seems risky and isn't what the name and function comment
>
> OK, how about this delta from the previous patch to ameliorate the
> maintenance risk?

attr_value_alignment seems clearer, and means that we can handle
things like:

  (mult (symbol_ref "...") (const_int 4))

How about the patch below?

Thanks,
Richard


2019-01-04  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* genattrtab.c (or_attr_value): Replace with...
	(attr_value_alignment): ...this new function.  Handle PLUS,
	MINUS and MULT.
	(write_length_unit_log): Update accordingly.

Index: gcc/genattrtab.c
===================================================================
--- gcc/genattrtab.c	2019-01-04 12:16:51.000000000 +0000
+++ gcc/genattrtab.c	2019-01-04 12:16:51.322900008 +0000
@@ -268,7 +268,7 @@ static void insert_insn_ent        (stru
 static void walk_attr_value	   (rtx);
 static int max_attr_value	   (rtx, int*);
 static int min_attr_value	   (rtx, int*);
-static int or_attr_value	   (rtx, int*);
+static unsigned int attr_value_alignment (rtx);
 static rtx simplify_test_exp	   (rtx, int, int);
 static rtx simplify_test_exp_in_temp (rtx, int, int);
 static rtx copy_rtx_unchanging	   (rtx);
@@ -1568,24 +1568,21 @@ write_length_unit_log (FILE *outf)
   struct attr_value *av;
   struct insn_ent *ie;
   unsigned int length_unit_log, length_or;
-  int unknown = 0;
 
   if (length_attr)
     {
-      length_or = or_attr_value (length_attr->default_val->value, &unknown);
+      length_or = attr_value_alignment (length_attr->default_val->value);
       for (av = length_attr->first_value; av; av = av->next)
 	for (ie = av->first_insn; ie; ie = ie->next)
-	  length_or |= or_attr_value (av->value, &unknown);
-    }
+	  length_or |= attr_value_alignment (av->value);
 
-  if (length_attr == NULL || unknown)
-    length_unit_log = 0;
-  else
-    {
       length_or = ~length_or;
       for (length_unit_log = 0; length_or & 1; length_or >>= 1)
 	length_unit_log++;
     }
+  else
+    length_unit_log = 0;
+
   fprintf (outf, "EXPORTED_CONST int length_unit_log = %u;\n", length_unit_log);
 }
 
@@ -3835,16 +3832,16 @@ min_attr_value (rtx exp, int *unknownp)
   return current_min;
 }
 
-/* Given an attribute value, return the result of ORing together all
-   CONST_STRING arguments encountered.  Set *UNKNOWNP and return -1
-   if the numeric value is not known.  */
+/* Given an attribute value EXP, return the maximum alignment that all
+   values are known to have.  Return 0 if EXP is known to be zero.  */
 
-static int
-or_attr_value (rtx exp, int *unknownp)
+static unsigned int
+attr_value_alignment (rtx exp)
 {
-  int current_or;
+  unsigned int current_or;
   int i;
 
+  /* When breaking, OR the possible alignment values into CURRENT_OR.  */
   switch (GET_CODE (exp))
     {
     case CONST_STRING:
@@ -3852,23 +3849,31 @@ or_attr_value (rtx exp, int *unknownp)
       break;
 
     case COND:
-      current_or = or_attr_value (XEXP (exp, 1), unknownp);
+      current_or = attr_value_alignment (XEXP (exp, 1));
       for (i = 0; i < XVECLEN (exp, 0); i += 2)
-	current_or |= or_attr_value (XVECEXP (exp, 0, i + 1), unknownp);
+	current_or |= attr_value_alignment (XVECEXP (exp, 0, i + 1));
       break;
 
     case IF_THEN_ELSE:
-      current_or = or_attr_value (XEXP (exp, 1), unknownp);
-      current_or |= or_attr_value (XEXP (exp, 2), unknownp);
+      current_or = attr_value_alignment (XEXP (exp, 1));
+      current_or |= attr_value_alignment (XEXP (exp, 2));
       break;
 
-    default:
-      *unknownp = 1;
-      current_or = -1;
+    case PLUS:
+    case MINUS:
+      current_or = attr_value_alignment (XEXP (exp, 0));
+      current_or |= attr_value_alignment (XEXP (exp, 1));
       break;
+
+    case MULT:
+      return (attr_value_alignment (XEXP (exp, 0))
+	      * attr_value_alignment (XEXP (exp, 1)));
+
+    default:
+      return 1;
     }
 
-  return current_or;
+  return current_or & -current_or;
 }
 
 /* Scan an attribute value, possibly a conditional, and record what actions