[AARCH64,3/3] AArch64 Port
diff mbox

Message ID 4FBF6AB0.4040007@arm.com
State New
Headers show

Commit Message

Marcus Shawcroft May 25, 2012, 11:19 a.m. UTC
This patch adds an implementation of integer iterators.

Index: gcc/ChangeLog.aarch64

	* read-rtl.c (rtx_list): New data structure.
	(int_iterator_mapping): New data structure.
	(int_iterator_data): New. List of int iterator details.
	(num_int_iterator_data): New.
	(ints): New group list.
	(find_int): New. Find an int iterator in a list.
	(dummy_uses_int_iterator): Dummy handle.
	(dummy_apply_int_iterator): Dummy handle.
	(uses_int_iterator_p): New.
	(apply_iterator_to_rtx): Handle case for rtx field specifier 'i'.
	(initialize_iterators): Initialize int iterators data struts.
	(find_int_iterator): New. Find an Int iterators from a hash-table.
	(add_int_iterator: Add int iterator to database.
	(read_rtx): Parse and read int iterators mapping and attributes.
	Initialize int iterators group's hash-table. Memory management.
	(read_rtx_code): Handle case for rtl field specifier 'i'.

Comments

Steven Bosscher May 25, 2012, 1:07 p.m. UTC | #1
On Fri, May 25, 2012 at 1:19 PM, Marcus Shawcroft
<marcus.shawcroft@arm.com> wrote:
> This patch adds an implementation of integer iterators.
>
> Index: gcc/ChangeLog.aarch64
>
>        * read-rtl.c (rtx_list): New data structure.
>        (int_iterator_mapping): New data structure.
>        (int_iterator_data): New. List of int iterator details.
>        (num_int_iterator_data): New.
>        (ints): New group list.
>        (find_int): New. Find an int iterator in a list.
>        (dummy_uses_int_iterator): Dummy handle.
>        (dummy_apply_int_iterator): Dummy handle.
>        (uses_int_iterator_p): New.
>        (apply_iterator_to_rtx): Handle case for rtx field specifier 'i'.
>        (initialize_iterators): Initialize int iterators data struts.
>        (find_int_iterator): New. Find an Int iterators from a hash-table.
>        (add_int_iterator: Add int iterator to database.
>        (read_rtx): Parse and read int iterators mapping and attributes.
>        Initialize int iterators group's hash-table. Memory management.
>        (read_rtx_code): Handle case for rtl field specifier 'i'.

Hello,

Can you please use diff -up (or svn diff -x -up) when you post a
patch? With -p it is easier to see what functions you have modified.

Can you please add documentation for this new iterator (I suppose in
doc/md.texi??).

> @@ -480,6 +563,7 @@
>
>    iterator = (struct mapping *) *slot;
>    for (elem = mtd->queue; elem != 0; elem = XEXP (elem, 1))
> +  {
>      if (uses_iterator_p (XEXP (elem, 0), iterator))
>        {
>  	/* For each iterator we expand, we set UNKNOWN_MODE_ATTR to NULL.
> @@ -509,6 +593,7 @@
>  	    XEXP (elem, 0) = x;
>  	  }
>      }
> +  }
>    return 1;
>  }

Not sure what this change is for. In any case, indentation does not
conform to GCC coding conventions (I don't think everyone is
particularly fond of them, but we should follow them nonetheless ;-)

Ciao!
Steven
Richard Sandiford May 27, 2012, 9:49 a.m. UTC | #2
Marcus Shawcroft <marcus.shawcroft@arm.com> writes:
> This patch adds an implementation of integer iterators.

Nice.  A few comments from an onlooker (on top of what Stephen said).

> +/* Since GCC does not construct a table of valid constants,
> +   we have to accept any int as valid.  No cross-checking can
> +   be done.  */
> +static int
> +find_int (const char *name)
> +{
> +  char *endptr;
> +  int ret;
> +
> +  if (ISDIGIT (*name))
> +    {
> +      ret = strtol (name, &endptr, 0);
> +      gcc_assert (*endptr == '\0');

I think this should be an error rather than an assert.

> +/* Stand-alone int iterator usage-checking function.  */
> +static bool
> +uses_int_iterator_p (rtx x, struct mapping *iterator, int opno)
> +{
> +  int i;
> +  for (i=0; i < num_int_iterator_data; i++)
> +    if (int_iterator_data[i].iterator->group == iterator->group &&
> +	int_iterator_data[i].iterator->index == iterator->index)

Formatting: && should be at the beginning of the second line.

> +      {
> +	/* Found an existing entry. Check if X is in its list.  */
> +	struct int_iterator_mapping it = int_iterator_data[i];
> +	int j;
> +
> +	for (j=0; j < it.num_rtx; j++)
> +	{
> +	  if (it.rtxs[j].x == x && it.rtxs[j].opno == opno)
> +	    return true;
> +	}

Formatting: redundant { ... }.

It might be easier to store a pointer to XEXP (x, opno) than storing
x and opno separately.

> +      }
> +  return false;
> +}
> +
>  /* Map a code or mode attribute string P to the underlying string for
>     ITERATOR and VALUE.  */
>  
> @@ -341,7 +414,9 @@
>    x = rtx_alloc (bellwether_code);
>    memcpy (x, original, RTX_CODE_SIZE (bellwether_code));
>  
> -  /* Change the mode or code itself.  */
> +  /* Change the mode or code itself.
> +     For int iterators, apply_iterator () does nothing. This is
> +     because we want to apply int iterators to operands below.  */

The way I imagined this working is that we'd just walk a list of
rtx * pointers for the current iterator and substitute the current
iterator value.  Then we'd take a deep copy of the rtx once all
iterators had been applied.  Checking every operand against the
substitution table seems a bit round-about.

It'd be good to do the same for codes and modes, but I'll volunteer
to do that as a follow-up.

> +/* Add to triplet-database for int iterators.  */
> +static void
> +add_int_iterator (struct mapping *iterator, rtx x, int opno)
> +{
> +
> +  /* Find iterator in int_iterator_data. If already present,
> +     add this R to its list of rtxs. If not present, create
> +     a new entry for INT_ITERATOR_DATA and add the R to its
> +     rtx list.  */
> +  int i;
> +  for (i=0; i < num_int_iterator_data; i++)
> +    if (int_iterator_data[i].iterator->index == iterator->index)
> +      {
> +	/* Found an existing entry. Add rtx to this iterator's list.  */
> +	int_iterator_data[i].rtxs =
> +			XRESIZEVEC (struct rtx_list,
> +				    int_iterator_data[i].rtxs,
> +				    int_iterator_data[i].num_rtx + 1);
> +	int_iterator_data[i].rtxs[int_iterator_data[i].num_rtx].x = x;
> +	int_iterator_data[i].rtxs[int_iterator_data[i].num_rtx].opno = opno;
> +	int_iterator_data[i].num_rtx++;
> +	return;
> +      }
> +
> +  /* New INT_ITERATOR_DATA entry.  */
> +  if (num_int_iterator_data == 0)
> +    int_iterator_data = XNEWVEC (struct int_iterator_mapping, 1);
> +  else
> +    int_iterator_data = XRESIZEVEC (struct int_iterator_mapping,
> +				    int_iterator_data,
> +				    num_int_iterator_data + 1);
> +  int_iterator_data[num_int_iterator_data].iterator = iterator;
> +  int_iterator_data[num_int_iterator_data].rtxs = XNEWVEC (struct rtx_list, 1);
> +  int_iterator_data[num_int_iterator_data].rtxs[0].x = x;
> +  int_iterator_data[num_int_iterator_data].rtxs[0].opno = opno;
> +  int_iterator_data[num_int_iterator_data].num_rtx = 1;
> +  num_int_iterator_data++;
> +}

VECs might be better here.

> @@ -1057,14 +1227,30 @@
>  	XWINT (return_rtx, i) = tmp_wide;
>  	break;
>  
> -      case 'i':
>        case 'n':
> -	read_name (&name);
>  	validate_const_int (name.string);
>  	tmp_int = atoi (name.string);
>  	XINT (return_rtx, i) = tmp_int;
>  	break;
> -
> +      case 'i':
> +	/* Can be an iterator or an integer constant.  */
> +	read_name (&name);
> +	if (!ISDIGIT (name.string[0]))
> +	  {
> +	    struct mapping *iterator;
> +	    /* An iterator.  */
> +	    iterator = find_int_iterator (&ints, name.string);
> +	    /* Build (iterator, rtx, op) triplet-database.  */
> +	    add_int_iterator (iterator, return_rtx, i);
> +	  }
> +	else
> +	  {
> +	    /* A numeric constant.  */
> +	    validate_const_int (name.string);
> +	    tmp_int = atoi (name.string);
> +	    XINT (return_rtx, i) = tmp_int;
> +	  }
> +	break;
>        default:
>  	gcc_unreachable ();
>        }

I don't see any need to split "i" and "n".  In fact we probably
shouldn't handle "n" at all, since it's only used in NOTE_INSNs.
So I think we should either remove the "n" case or continue to
treat it in the same way as "i".

Richard
Tejas Belagod May 28, 2012, 12:49 p.m. UTC | #3
Hi Richard,

Thanks for your comments. Some questions inline below.

Richard Sandiford wrote:
> Marcus Shawcroft <marcus.shawcroft@arm.com> writes:
>> This patch adds an implementation of integer iterators.
> 
> Nice.  A few comments from an onlooker (on top of what Stephen said).
> 
>> +/* Since GCC does not construct a table of valid constants,
>> +   we have to accept any int as valid.  No cross-checking can
>> +   be done.  */
>> +static int
>> +find_int (const char *name)
>> +{
>> +  char *endptr;
>> +  int ret;
>> +
>> +  if (ISDIGIT (*name))
>> +    {
>> +      ret = strtol (name, &endptr, 0);
>> +      gcc_assert (*endptr == '\0');
> 
> I think this should be an error rather than an assert.
> 
>> +/* Stand-alone int iterator usage-checking function.  */
>> +static bool
>> +uses_int_iterator_p (rtx x, struct mapping *iterator, int opno)
>> +{
>> +  int i;
>> +  for (i=0; i < num_int_iterator_data; i++)
>> +    if (int_iterator_data[i].iterator->group == iterator->group &&
>> +	int_iterator_data[i].iterator->index == iterator->index)
> 
> Formatting: && should be at the beginning of the second line.
> 
>> +      {
>> +	/* Found an existing entry. Check if X is in its list.  */
>> +	struct int_iterator_mapping it = int_iterator_data[i];
>> +	int j;
>> +
>> +	for (j=0; j < it.num_rtx; j++)
>> +	{
>> +	  if (it.rtxs[j].x == x && it.rtxs[j].opno == opno)
>> +	    return true;
>> +	}
> 
> Formatting: redundant { ... }.
> 
> It might be easier to store a pointer to XEXP (x, opno) than storing
> x and opno separately.
> 
>> +      }
>> +  return false;
>> +}
>> +
>>  /* Map a code or mode attribute string P to the underlying string for
>>     ITERATOR and VALUE.  */
>>  
>> @@ -341,7 +414,9 @@
>>    x = rtx_alloc (bellwether_code);
>>    memcpy (x, original, RTX_CODE_SIZE (bellwether_code));
>>  
>> -  /* Change the mode or code itself.  */
>> +  /* Change the mode or code itself.
>> +     For int iterators, apply_iterator () does nothing. This is
>> +     because we want to apply int iterators to operands below.  */
> 
> The way I imagined this working is that we'd just walk a list of
> rtx * pointers for the current iterator and substitute the current
> iterator value.  Then we'd take a deep copy of the rtx once all
> iterators had been applied.  Checking every operand against the
> substitution table seems a bit round-about.
> 

I understand how this would work for mode and code iterators, but I'm a 
bit confused about how it would for int iterators. Don't we have to 
traverse each operand to figure out which ones to substitute for an int 
iterator value? Also, when you say take a deep copy after all the 
iterators have been applied, do you mean code, mode and int iterators or 
do you mean values of a particular iterator? As I understand the current 
implementation, mode and code iterators use placeholder integral 
constants that are replaced with actual iterator values during the rtx 
traverse. If we take a deep copy after the replacement, won't we lose 
these placeholder codes?

Thanks,
Tejas.

> It'd be good to do the same for codes and modes, but I'll volunteer
> to do that as a follow-up.
> 
>> +/* Add to triplet-database for int iterators.  */
>> +static void
>> +add_int_iterator (struct mapping *iterator, rtx x, int opno)
>> +{
>> +
>> +  /* Find iterator in int_iterator_data. If already present,
>> +     add this R to its list of rtxs. If not present, create
>> +     a new entry for INT_ITERATOR_DATA and add the R to its
>> +     rtx list.  */
>> +  int i;
>> +  for (i=0; i < num_int_iterator_data; i++)
>> +    if (int_iterator_data[i].iterator->index == iterator->index)
>> +      {
>> +	/* Found an existing entry. Add rtx to this iterator's list.  */
>> +	int_iterator_data[i].rtxs =
>> +			XRESIZEVEC (struct rtx_list,
>> +				    int_iterator_data[i].rtxs,
>> +				    int_iterator_data[i].num_rtx + 1);
>> +	int_iterator_data[i].rtxs[int_iterator_data[i].num_rtx].x = x;
>> +	int_iterator_data[i].rtxs[int_iterator_data[i].num_rtx].opno = opno;
>> +	int_iterator_data[i].num_rtx++;
>> +	return;
>> +      }
>> +
>> +  /* New INT_ITERATOR_DATA entry.  */
>> +  if (num_int_iterator_data == 0)
>> +    int_iterator_data = XNEWVEC (struct int_iterator_mapping, 1);
>> +  else
>> +    int_iterator_data = XRESIZEVEC (struct int_iterator_mapping,
>> +				    int_iterator_data,
>> +				    num_int_iterator_data + 1);
>> +  int_iterator_data[num_int_iterator_data].iterator = iterator;
>> +  int_iterator_data[num_int_iterator_data].rtxs = XNEWVEC (struct rtx_list, 1);
>> +  int_iterator_data[num_int_iterator_data].rtxs[0].x = x;
>> +  int_iterator_data[num_int_iterator_data].rtxs[0].opno = opno;
>> +  int_iterator_data[num_int_iterator_data].num_rtx = 1;
>> +  num_int_iterator_data++;
>> +}
> 
> VECs might be better here.
> 
>> @@ -1057,14 +1227,30 @@
>>  	XWINT (return_rtx, i) = tmp_wide;
>>  	break;
>>  
>> -      case 'i':
>>        case 'n':
>> -	read_name (&name);
>>  	validate_const_int (name.string);
>>  	tmp_int = atoi (name.string);
>>  	XINT (return_rtx, i) = tmp_int;
>>  	break;
>> -
>> +      case 'i':
>> +	/* Can be an iterator or an integer constant.  */
>> +	read_name (&name);
>> +	if (!ISDIGIT (name.string[0]))
>> +	  {
>> +	    struct mapping *iterator;
>> +	    /* An iterator.  */
>> +	    iterator = find_int_iterator (&ints, name.string);
>> +	    /* Build (iterator, rtx, op) triplet-database.  */
>> +	    add_int_iterator (iterator, return_rtx, i);
>> +	  }
>> +	else
>> +	  {
>> +	    /* A numeric constant.  */
>> +	    validate_const_int (name.string);
>> +	    tmp_int = atoi (name.string);
>> +	    XINT (return_rtx, i) = tmp_int;
>> +	  }
>> +	break;
>>        default:
>>  	gcc_unreachable ();
>>        }
> 
> I don't see any need to split "i" and "n".  In fact we probably
> shouldn't handle "n" at all, since it's only used in NOTE_INSNs.
> So I think we should either remove the "n" case or continue to
> treat it in the same way as "i".
> 
> Richard
>
Richard Sandiford May 28, 2012, 5:14 p.m. UTC | #4
Tejas Belagod <tbelagod@arm.com> writes:
> Hi Richard,
>
> Thanks for your comments. Some questions inline below.
>
> Richard Sandiford wrote:
>> Marcus Shawcroft <marcus.shawcroft@arm.com> writes:
>>> This patch adds an implementation of integer iterators.
>> 
>> Nice.  A few comments from an onlooker (on top of what Stephen said).
>> 
>>> +/* Since GCC does not construct a table of valid constants,
>>> +   we have to accept any int as valid.  No cross-checking can
>>> +   be done.  */
>>> +static int
>>> +find_int (const char *name)
>>> +{
>>> +  char *endptr;
>>> +  int ret;
>>> +
>>> +  if (ISDIGIT (*name))
>>> +    {
>>> +      ret = strtol (name, &endptr, 0);
>>> +      gcc_assert (*endptr == '\0');
>> 
>> I think this should be an error rather than an assert.
>> 
>>> +/* Stand-alone int iterator usage-checking function.  */
>>> +static bool
>>> +uses_int_iterator_p (rtx x, struct mapping *iterator, int opno)
>>> +{
>>> +  int i;
>>> +  for (i=0; i < num_int_iterator_data; i++)
>>> +    if (int_iterator_data[i].iterator->group == iterator->group &&
>>> +	int_iterator_data[i].iterator->index == iterator->index)
>> 
>> Formatting: && should be at the beginning of the second line.
>> 
>>> +      {
>>> +	/* Found an existing entry. Check if X is in its list.  */
>>> +	struct int_iterator_mapping it = int_iterator_data[i];
>>> +	int j;
>>> +
>>> +	for (j=0; j < it.num_rtx; j++)
>>> +	{
>>> +	  if (it.rtxs[j].x == x && it.rtxs[j].opno == opno)
>>> +	    return true;
>>> +	}
>> 
>> Formatting: redundant { ... }.
>> 
>> It might be easier to store a pointer to XEXP (x, opno) than storing
>> x and opno separately.
>> 
>>> +      }
>>> +  return false;
>>> +}
>>> +
>>>  /* Map a code or mode attribute string P to the underlying string for
>>>     ITERATOR and VALUE.  */
>>>  
>>> @@ -341,7 +414,9 @@
>>>    x = rtx_alloc (bellwether_code);
>>>    memcpy (x, original, RTX_CODE_SIZE (bellwether_code));
>>>  
>>> -  /* Change the mode or code itself.  */
>>> +  /* Change the mode or code itself.
>>> +     For int iterators, apply_iterator () does nothing. This is
>>> +     because we want to apply int iterators to operands below.  */
>> 
>> The way I imagined this working is that we'd just walk a list of
>> rtx * pointers for the current iterator and substitute the current
>> iterator value.  Then we'd take a deep copy of the rtx once all
>> iterators had been applied.  Checking every operand against the
>> substitution table seems a bit round-about.
>> 
>
> I understand how this would work for mode and code iterators, but I'm a 
> bit confused about how it would for int iterators.

Probably because of a typo, sorry.  I meant "int *" in the above.
At least, they'd be "int *" for int iterators and "rtx" for mode
and code iterators.

> Don't we have to 
> traverse each operand to figure out which ones to substitute for an int 
> iterator value? Also, when you say take a deep copy after all the 
> iterators have been applied, do you mean code, mode and int iterators or 
> do you mean values of a particular iterator? As I understand the current 
> implementation, mode and code iterators use placeholder integral 
> constants that are replaced with actual iterator values during the rtx 
> traverse. If we take a deep copy after the replacement, won't we lose 
> these placeholder codes?

If you don't convert codes and modes (and leave it to me), then you'd
probably need to apply all int interators first.  I expect it'd be easier
to convert modes and codes at the same time.

Richard
Tejas Belagod May 31, 2012, 10:49 a.m. UTC | #5
Richard Sandiford wrote:
> Tejas Belagod <tbelagod@arm.com> writes:
>> Hi Richard,
>>
>> Thanks for your comments. Some questions inline below.
>>
>> Richard Sandiford wrote:
>>> Marcus Shawcroft <marcus.shawcroft@arm.com> writes:
>>>> This patch adds an implementation of integer iterators.
>>> Nice.  A few comments from an onlooker (on top of what Stephen said).
>>>
>>>> +/* Since GCC does not construct a table of valid constants,
>>>> +   we have to accept any int as valid.  No cross-checking can
>>>> +   be done.  */
>>>> +static int
>>>> +find_int (const char *name)
>>>> +{
>>>> +  char *endptr;
>>>> +  int ret;
>>>> +
>>>> +  if (ISDIGIT (*name))
>>>> +    {
>>>> +      ret = strtol (name, &endptr, 0);
>>>> +      gcc_assert (*endptr == '\0');
>>> I think this should be an error rather than an assert.
>>>
>>>> +/* Stand-alone int iterator usage-checking function.  */
>>>> +static bool
>>>> +uses_int_iterator_p (rtx x, struct mapping *iterator, int opno)
>>>> +{
>>>> +  int i;
>>>> +  for (i=0; i < num_int_iterator_data; i++)
>>>> +    if (int_iterator_data[i].iterator->group == iterator->group &&
>>>> +	int_iterator_data[i].iterator->index == iterator->index)
>>> Formatting: && should be at the beginning of the second line.
>>>
>>>> +      {
>>>> +	/* Found an existing entry. Check if X is in its list.  */
>>>> +	struct int_iterator_mapping it = int_iterator_data[i];
>>>> +	int j;
>>>> +
>>>> +	for (j=0; j < it.num_rtx; j++)
>>>> +	{
>>>> +	  if (it.rtxs[j].x == x && it.rtxs[j].opno == opno)
>>>> +	    return true;
>>>> +	}
>>> Formatting: redundant { ... }.
>>>
>>> It might be easier to store a pointer to XEXP (x, opno) than storing
>>> x and opno separately.
>>>
>>>> +      }
>>>> +  return false;
>>>> +}
>>>> +
>>>>  /* Map a code or mode attribute string P to the underlying string for
>>>>     ITERATOR and VALUE.  */
>>>>  
>>>> @@ -341,7 +414,9 @@
>>>>    x = rtx_alloc (bellwether_code);
>>>>    memcpy (x, original, RTX_CODE_SIZE (bellwether_code));
>>>>  
>>>> -  /* Change the mode or code itself.  */
>>>> +  /* Change the mode or code itself.
>>>> +     For int iterators, apply_iterator () does nothing. This is
>>>> +     because we want to apply int iterators to operands below.  */
>>> The way I imagined this working is that we'd just walk a list of
>>> rtx * pointers for the current iterator and substitute the current
>>> iterator value.  Then we'd take a deep copy of the rtx once all
>>> iterators had been applied.  Checking every operand against the
>>> substitution table seems a bit round-about.
>>>
>> I understand how this would work for mode and code iterators, but I'm a 
>> bit confused about how it would for int iterators.
> 
> Probably because of a typo, sorry.  I meant "int *" in the above.
> At least, they'd be "int *" for int iterators and "rtx" for mode
> and code iterators.
> 

Thanks for the clarification.

>> Don't we have to 
>> traverse each operand to figure out which ones to substitute for an int 
>> iterator value? Also, when you say take a deep copy after all the 
>> iterators have been applied, do you mean code, mode and int iterators or 
>> do you mean values of a particular iterator? As I understand the current 
>> implementation, mode and code iterators use placeholder integral 
>> constants that are replaced with actual iterator values during the rtx 
>> traverse. If we take a deep copy after the replacement, won't we lose 
>> these placeholder codes?
> 
> If you don't convert codes and modes (and leave it to me), then you'd
> probably need to apply all int interators first.  I expect it'd be easier
> to convert modes and codes at the same time.

In the currect scheme, when multiple code/mode iterators are in an rtx pattern, 
they are expanded for each combination of iterator values in 
apply_iterator_traverse () and a repeated traversal of the expanded rtx's for 
each iterator achieves the 'cross-product' of rtx's for iterator value 
combinations. This allows for having the place-holder codes/modes in new rtx's 
that are shallow-copied and a subsequent traversal will eventually replace them 
with actual code/mode iterator values. Doing away with the placeholders means 
that we have to build a 'cross-product' of iterator values from a database of 
iterator values as used in a pattern and make an rtx copy(deep) corresponding to 
each element of the cross-product with the iterator value combinations 
substituted in. This is my understanding of the new implementation - am I on the 
right track?

Thanks,
Tejas.
Richard Sandiford May 31, 2012, 9:21 p.m. UTC | #6
Tejas Belagod <tbelagod@arm.com> writes:
> In the currect scheme, when multiple code/mode iterators are in an rtx pattern, 
> they are expanded for each combination of iterator values in 
> apply_iterator_traverse () and a repeated traversal of the expanded rtx's for 
> each iterator achieves the 'cross-product' of rtx's for iterator value 
> combinations. This allows for having the place-holder codes/modes in new rtx's 
> that are shallow-copied and a subsequent traversal will eventually replace them 
> with actual code/mode iterator values. Doing away with the placeholders means 
> that we have to build a 'cross-product' of iterator values from a database of 
> iterator values as used in a pattern and make an rtx copy(deep) corresponding to 
> each element of the cross-product with the iterator value combinations 
> substituted in. This is my understanding of the new implementation - am I on the 
> right track?

That makes it sound a lot more complicated than it was supposed to be. :-)

I'm testing a patch to convert the modes and codes now.  Hopefully it'll
be easy to add your stuff on top of that.

Richard

Patch
diff mbox

Index: gcc/read-rtl.c
===================================================================
--- gcc/read-rtl.c	(revision 187870)
+++ gcc/read-rtl.c	(working copy)
@@ -94,6 +94,26 @@ 
 #define BELLWETHER_CODE(CODE) \
   ((CODE) < NUM_RTX_CODE ? CODE : bellwether_codes[CODE - NUM_RTX_CODE])
 
+/* One element in the (rtx, opno) pair list.  */
+struct rtx_list {
+  /* rtx.  */
+  rtx x;
+  /* Position of the operand to replace.  */
+  int opno;
+};
+
+/* A structure to track which rtx uses which int iterator.  */
+struct int_iterator_mapping {
+  /* Iterator.  */
+  struct mapping *iterator;
+  /* list of rtx using ITERATOR.  */
+  struct rtx_list *rtxs;
+  int num_rtx;
+};
+
+static struct int_iterator_mapping *int_iterator_data;
+static int num_int_iterator_data;
+
 static int find_mode (const char *);
 static bool uses_mode_iterator_p (rtx, int);
 static void apply_mode_iterator (rtx, int);
@@ -121,8 +141,8 @@ 
 static rtx read_nested_rtx (struct map_value **);
 static rtx read_rtx_variadic (struct map_value **, rtx);
 
-/* The mode and code iterator structures.  */
-static struct iterator_group modes, codes;
+/* The mode, code and int iterator structures.  */
+static struct iterator_group modes, codes, ints;
 
 /* Index I is the value of BELLWETHER_CODE (I + NUM_RTX_CODE).  */
 static enum rtx_code *bellwether_codes;
@@ -179,6 +199,59 @@ 
   PUT_CODE (x, (enum rtx_code) code);
 }
 
+/* Since GCC does not construct a table of valid constants,
+   we have to accept any int as valid.  No cross-checking can
+   be done.  */
+static int
+find_int (const char *name)
+{
+  char *endptr;
+  int ret;
+
+  if (ISDIGIT (*name))
+    {
+      ret = strtol (name, &endptr, 0);
+      gcc_assert (*endptr == '\0');
+      return ret;
+    }
+  else
+    fatal_with_file_and_line ("unknown int `%s'", name);
+}
+
+static bool
+dummy_uses_int_iterator (rtx x ATTRIBUTE_UNUSED, int index ATTRIBUTE_UNUSED)
+{
+  return false;
+}
+
+static void
+dummy_apply_int_iterator (rtx x ATTRIBUTE_UNUSED, int code ATTRIBUTE_UNUSED)
+{
+  /* Do nothing.  */
+}
+
+/* Stand-alone int iterator usage-checking function.  */
+static bool
+uses_int_iterator_p (rtx x, struct mapping *iterator, int opno)
+{
+  int i;
+  for (i=0; i < num_int_iterator_data; i++)
+    if (int_iterator_data[i].iterator->group == iterator->group &&
+	int_iterator_data[i].iterator->index == iterator->index)
+      {
+	/* Found an existing entry. Check if X is in its list.  */
+	struct int_iterator_mapping it = int_iterator_data[i];
+	int j;
+
+	for (j=0; j < it.num_rtx; j++)
+	{
+	  if (it.rtxs[j].x == x && it.rtxs[j].opno == opno)
+	    return true;
+	}
+      }
+  return false;
+}
+
 /* Map a code or mode attribute string P to the underlying string for
    ITERATOR and VALUE.  */
 
@@ -341,7 +414,9 @@ 
   x = rtx_alloc (bellwether_code);
   memcpy (x, original, RTX_CODE_SIZE (bellwether_code));
 
-  /* Change the mode or code itself.  */
+  /* Change the mode or code itself.
+     For int iterators, apply_iterator () does nothing. This is
+     because we want to apply int iterators to operands below.  */
   group = iterator->group;
   if (group->uses_iterator_p (x, iterator->index + group->num_builtins))
     group->apply_iterator (x, value);
@@ -379,6 +454,10 @@ 
 							 unknown_mode_attr);
 	  }
 	break;
+      case 'i':
+	if (uses_int_iterator_p (original, iterator, i))
+	  XINT (x, i) = value;
+	break;
 
       default:
 	break;
@@ -419,6 +498,10 @@ 
 	      return true;
 	break;
 
+      case 'i':
+	if (uses_int_iterator_p (x, iterator, i))
+	  return true;
+
       default:
 	break;
       }
@@ -480,6 +563,7 @@ 
 
   iterator = (struct mapping *) *slot;
   for (elem = mtd->queue; elem != 0; elem = XEXP (elem, 1))
+  {
     if (uses_iterator_p (XEXP (elem, 0), iterator))
       {
 	/* For each iterator we expand, we set UNKNOWN_MODE_ATTR to NULL.
@@ -509,6 +593,7 @@ 
 	    XEXP (elem, 0) = x;
 	  }
     }
+  }
   return 1;
 }
 
@@ -553,7 +638,7 @@ 
   return &value->next;
 }
 
-/* Do one-time initialization of the mode and code attributes.  */
+/* Do one-time initialization of the mode, code and int attributes.  */
 
 static void
 initialize_iterators (void)
@@ -579,6 +664,15 @@ 
   codes.uses_iterator_p = uses_code_iterator_p;
   codes.apply_iterator = apply_code_iterator;
 
+  ints.attrs = htab_create (13, leading_string_hash, leading_string_eq_p, 0);
+  ints.iterators = htab_create (13, leading_string_hash,
+				leading_string_eq_p, 0);
+  ints.num_builtins = 0;
+  ints.find_builtin = find_int;
+  ints.uses_iterator_p = dummy_uses_int_iterator;
+  ints.apply_iterator = dummy_apply_int_iterator;
+  num_int_iterator_data = 0;
+
   lower = add_mapping (&modes, modes.attrs, "mode");
   upper = add_mapping (&modes, modes.attrs, "MODE");
   lower_ptr = &lower->values;
@@ -728,6 +822,61 @@ 
   return group->find_builtin (name);
 }
 
+/* We cannot use the same design as code and mode iterators as ints
+   can be any arbitrary number and there is no way to represent each
+   int iterator's placeholder with a unique numeric identifier. Therefore
+   we create a (rtx *, op, iterator *) triplet database.  */
+
+static struct mapping *
+find_int_iterator (struct iterator_group *group, const char *name)
+{
+  struct mapping *m;
+
+  m = (struct mapping *) htab_find (group->iterators, &name);
+  if (m == 0)
+    fatal_with_file_and_line ("invalid iterator \"%s\"\n", name);
+  return m;
+}
+
+/* Add to triplet-database for int iterators.  */
+static void
+add_int_iterator (struct mapping *iterator, rtx x, int opno)
+{
+
+  /* Find iterator in int_iterator_data. If already present,
+     add this R to its list of rtxs. If not present, create
+     a new entry for INT_ITERATOR_DATA and add the R to its
+     rtx list.  */
+  int i;
+  for (i=0; i < num_int_iterator_data; i++)
+    if (int_iterator_data[i].iterator->index == iterator->index)
+      {
+	/* Found an existing entry. Add rtx to this iterator's list.  */
+	int_iterator_data[i].rtxs =
+			XRESIZEVEC (struct rtx_list,
+				    int_iterator_data[i].rtxs,
+				    int_iterator_data[i].num_rtx + 1);
+	int_iterator_data[i].rtxs[int_iterator_data[i].num_rtx].x = x;
+	int_iterator_data[i].rtxs[int_iterator_data[i].num_rtx].opno = opno;
+	int_iterator_data[i].num_rtx++;
+	return;
+      }
+
+  /* New INT_ITERATOR_DATA entry.  */
+  if (num_int_iterator_data == 0)
+    int_iterator_data = XNEWVEC (struct int_iterator_mapping, 1);
+  else
+    int_iterator_data = XRESIZEVEC (struct int_iterator_mapping,
+				    int_iterator_data,
+				    num_int_iterator_data + 1);
+  int_iterator_data[num_int_iterator_data].iterator = iterator;
+  int_iterator_data[num_int_iterator_data].rtxs = XNEWVEC (struct rtx_list, 1);
+  int_iterator_data[num_int_iterator_data].rtxs[0].x = x;
+  int_iterator_data[num_int_iterator_data].rtxs[0].opno = opno;
+  int_iterator_data[num_int_iterator_data].num_rtx = 1;
+  num_int_iterator_data++;
+}
+
 /* Finish reading a declaration of the form:
 
        (define... <name> [<value1> ... <valuen>])
@@ -817,6 +966,7 @@ 
   static rtx queue_head;
   struct map_value *mode_maps;
   struct iterator_traverse_data mtd;
+  int i;
 
   /* Do one-time initialization.  */
   if (queue_head == 0)
@@ -852,7 +1002,18 @@ 
       check_code_iterator (read_mapping (&codes, codes.iterators));
       return false;
     }
+  if (strcmp (rtx_name, "define_int_attr") == 0)
+    {
+      read_mapping (&ints, ints.attrs);
+      return false;
+    }
+  if (strcmp (rtx_name, "define_int_iterator") == 0)
+    {
+      read_mapping (&ints, ints.iterators);
+      return false;
+    }
 
+
   mode_maps = 0;
   XEXP (queue_head, 0) = read_rtx_code (rtx_name, &mode_maps);
   XEXP (queue_head, 1) = 0;
@@ -860,6 +1021,15 @@ 
   mtd.queue = queue_head;
   mtd.mode_maps = mode_maps;
   mtd.unknown_mode_attr = mode_maps ? mode_maps->string : NULL;
+  htab_traverse (ints.iterators, apply_iterator_traverse, &mtd);
+  /* Free used memory from recording int iterator usage.  */
+  for (i=0; i < num_int_iterator_data; i++)
+    if (int_iterator_data[i].num_rtx > 0)
+      XDELETEVEC (int_iterator_data[i].rtxs);
+  if (num_int_iterator_data > 0)
+    XDELETEVEC (int_iterator_data);
+  num_int_iterator_data = 0;
+
   htab_traverse (modes.iterators, apply_iterator_traverse, &mtd);
   htab_traverse (codes.iterators, apply_iterator_traverse, &mtd);
   if (mtd.unknown_mode_attr)
@@ -1057,14 +1227,30 @@ 
 	XWINT (return_rtx, i) = tmp_wide;
 	break;
 
-      case 'i':
       case 'n':
-	read_name (&name);
 	validate_const_int (name.string);
 	tmp_int = atoi (name.string);
 	XINT (return_rtx, i) = tmp_int;
 	break;
-
+      case 'i':
+	/* Can be an iterator or an integer constant.  */
+	read_name (&name);
+	if (!ISDIGIT (name.string[0]))
+	  {
+	    struct mapping *iterator;
+	    /* An iterator.  */
+	    iterator = find_int_iterator (&ints, name.string);
+	    /* Build (iterator, rtx, op) triplet-database.  */
+	    add_int_iterator (iterator, return_rtx, i);
+	  }
+	else
+	  {
+	    /* A numeric constant.  */
+	    validate_const_int (name.string);
+	    tmp_int = atoi (name.string);
+	    XINT (return_rtx, i) = tmp_int;
+	  }
+	break;
       default:
 	gcc_unreachable ();
       }