Re: RFA: Alternative iterator implementation

2012-06-12 Thread Tejas Belagod

Richard Sandiford wrote:

Thanks for the update.

Tejas Belagod tbela...@arm.com writes:

+/* Implementations of the iterator_group callbacks for ints.  */
+
+/* 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');


As I mentioned before, I think this should be an error rather than
an assert.  An assert would only be appropriate if this is something
that should already have been checked, but AFAICT nothing does.

In fact I think this ought to be:

  validate_const_int (name);
  return atoi (name);



Sorry, yes I meant to fix that. I've fixed that now.


And unless I'm missing something, this:


+   /* Can be an iterator or an integer constant.  */
read_name (name);
-   validate_const_int (name.string);
-   tmp_int = atoi (name.string);
-   XINT (return_rtx, i) = tmp_int;
+   if (!ISDIGIT (name.string[0]))
+ {
+   struct mapping *iterator;
+   /* An iterator.  */
+   iterator = (struct mapping *) htab_find (ints.iterators,
+name.string);
+   if (iterator)
+ record_iterator_use (iterator, XINT (return_rtx, i));
+   else
+ fatal_with_file_and_line (unknown iterator `%s',name.string);
+ }
+   else
+ {
+   validate_const_int (name.string);
+   tmp_int = atoi (name.string);
+   XINT (return_rtx, i) = tmp_int;
+ }


should simply be:

/* Can be an iterator or an integer constant.  */
read_name (name);
record_potential_iterator_use (ints, XINT (return_rtx, i),
   name.string);
break;


Indeed.

New patch attached. OK?

Thanks,
Tejas Belagod.
ARM.diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 71b89c1..94cd01b 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -8903,6 +8903,7 @@ facilities to make this process easier.
 @menu
 * Mode Iterators:: Generating variations of patterns for different 
modes.
 * Code Iterators:: Doing the same for codes.
+* Int Iterators::  Doing the same for integers.
 @end menu
 
 @node Mode Iterators
@@ -9174,4 +9175,83 @@ This is equivalent to:
 @dots{}
 @end smallexample
 
+@node Int Iterators
+@subsection Int Iterators
+@cindex int iterators in @file{.md} files
+@findex define_int_iterator
+@findex define_int_attr
+
+Int iterators operate in a similar way to code iterators.  @xref{Code 
Iterators}.
+
+The construct:
+
+@smallexample
+(define_int_iterator @var{name} [(@var{int1} @var{cond1}) @dots{} 
(@var{intn} @var{condn})])
+@end smallexample
+
+defines a pseudo integer constant @var{name} that can be instantiated as
+@var{inti} if condition @var{condi} is true.  Each @var{int}
+must have the same rtx format.  @xref{RTL Classes}. Int iterators can appear
+in only those rtx fields that have 'i' as the specifier. This means that
+each @var{int} has to be a constant defined using define_constant or
+define_c_enum.
+
+As with mode and code iterators, each pattern that uses @var{name} will be
+expanded @var{n} times, once with all uses of @var{name} replaced by
+@var{int1}, once with all uses replaced by @var{int2}, and so on.
+@xref{Defining Mode Iterators}.
+
+It is possible to define attributes for ints as well as for codes and modes.
+There are two standard integer attributes: @code{int}, the name of the
+code in lower case, and @code{INT}, the name of the code in upper case.
+Other attributes are defined using:
+
+@smallexample
+(define_int_attr @var{name} [(@var{int1} @var{value1}) @dots{} (@var{intn} 
@var{valuen})])
+@end smallexample
+
+Here's an example of int iterators in action, taken from the ARM port:
+
+@smallexample
+(define_int_iterator QABSNEG [UNSPEC_VQABS UNSPEC_VQNEG])
+
+(define_int_attr absneg [(UNSPEC_VQABS abs) (UNSPEC_VQNEG neg)])
+
+(define_insn neon_vqabsnegmode
+  [(set (match_operand:VDQIW 0 s_register_operand =w)
+   (unspec:VDQIW [(match_operand:VDQIW 1 s_register_operand w)
+  (match_operand:SI 2 immediate_operand i)]
+ QABSNEG))]
+  TARGET_NEON
+  vqabsneg.V_s_elem\t%V_reg0, %V_reg1
+  [(set_attr neon_type neon_vqneg_vqabs)]
+)
+
+@end smallexample
+
+This is equivalent to:
+
+@smallexample
+(define_insn neon_vqabsmode
+  [(set (match_operand:VDQIW 0 s_register_operand =w)
+   (unspec:VDQIW [(match_operand:VDQIW 1 s_register_operand w)
+  (match_operand:SI 2 immediate_operand i)]
+ UNSPEC_VQABS))]
+  TARGET_NEON
+  vqabs.V_s_elem\t%V_reg0, %V_reg1
+  [(set_attr neon_type neon_vqneg_vqabs)]
+)
+
+(define_insn neon_vqnegmode
+  [(set (match_operand:VDQIW 0 s_register_operand =w)
+   (unspec:VDQIW 

Re: RFA: Alternative iterator implementation

2012-06-12 Thread Richard Sandiford
Tejas Belagod tbela...@arm.com writes:
 New patch attached. OK?

 +There are two standard integer attributes: @code{int}, the name of the
 +code in lower case, and @code{INT}, the name of the code in upper case.

I don't think this is true.  So the surrounding paragraph reduces to:

  It is possible to define attributes for ints as well as for codes and modes.
  Attributes are defined using:

Looks good to me otherwise, thanks, although I can't approve it.
(I can't approve my own patch either, hint hint, ping ping.)

Thanks for your patience too.

Richard


Re: RFA: Alternative iterator implementation

2012-06-12 Thread Richard Henderson
On 2012-06-12 11:26, Richard Sandiford wrote:
 Tejas Belagod tbela...@arm.com writes:
 New patch attached. OK?
 
 +There are two standard integer attributes: @code{int}, the name of the
 +code in lower case, and @code{INT}, the name of the code in upper case.
 
 I don't think this is true.  So the surrounding paragraph reduces to:
 
   It is possible to define attributes for ints as well as for codes and modes.
   Attributes are defined using:
 
 Looks good to me otherwise, thanks, although I can't approve it.
 (I can't approve my own patch either, hint hint, ping ping.)
 
 Thanks for your patience too.

Both patches approved.


r~


Re: RFA: Alternative iterator implementation

2012-06-11 Thread Tejas Belagod

Richard Sandiford wrote:

As discussed in the context of the AARCH64 submission, this patch
rewrites the iterator handling in read-rtl.c so that we record
iterator positions using an on-the-side VEC rather than placeholder
modes and codes.  We then substitute in-place for each sequence of
iterator values and take a deep copy of the result.  We do any string
substitutions during the copy.

The patch also generalises the current use of attributes for rtx modes
((zero_extend:WIDER_MODE ...), etc.) so that the same kind of thing
can be done with future iterator types, including the int iterators.
Not sure whether that'll be useful or not, but it made the patch
easier to write.

Tested by making sure that the insn-*.c output didn't change for
x86_64-linux-gnu, except that we now do a better job of retaining
#line information (not seen as a good thing by everbody, I realise).
Also made sure that things like insn-output.c are still generated
in the blink of an eye.

Bootstrapped  regression-tested on x86_64-linux-gnu and i686-pc-linux-gnu.
OK to install?

Richard


gcc/
* read-rtl.c (mapping): Remove index field.  Add current_value field.
Define heap vectors.
(iterator_group): Fix long line.  Remove num_builtins field and
uses_iterator fields.  Make apply_iterator take a void * parameter.
(iterator_use, atttribute_use): New structures.
(iterator_traverse_data, BELLWETHER_CODE, bellwether_codes): Delete.
(current_iterators, iterator_uses, attribute_uses): New variables.
(uses_mode_iterator_p, uses_code_iterator_p): Delete.
(apply_mode_iterator, apply_code_iterator): Take a void * parameter.
(map_attr_string, apply_iterator_to_string): Remove iterator
and value parameters.  Look through all current iterator values
for a matching attribute.
(mode_attr_index, apply_mode_maps): Delete.
(apply_iterator_to_rtx): Replace with...
(copy_rtx_for_iterators): ...this new function.
(uses_iterator_p, apply_iterator_traverse): Delete.
(apply_attribute_uses, add_current_iterators, apply_iterators): New
functions.
(add_mapping): Remove index field.  Set current_value field.
(initialize_iterators): Don't set num_builtins and uses_iterator_p
fields.
(find_iterator): Delete.
(record_iterator_use, record_attribute_use): New functions.
(record_potential_iterator_use): New function.
(check_code_iterator): Remove handling of bellwether codes.
(read_rtx): Remove mode maps.  Truncate iterator and attribute uses.
(read_rtx_code, read_nested_rtx, read_rtx_variadic): Remove mode_maps
parameter.  Use the first code iterator value instead of the
bellwether_codes array.  Use record_potential_iterator_use
for modes.

Index: gcc/read-rtl.c
===
--- gcc/read-rtl.c  2012-06-03 08:58:32.251211521 +0100
+++ gcc/read-rtl.c  2012-06-03 09:20:47.633208254 +0100
@@ -41,7 +41,7 @@ struct map_value {
 };

 /* Maps an iterator or attribute name to a list of (integer, string) pairs.
-   The integers are mode or code values; the strings are either C conditions
+   The integers are iterator values; the strings are either C conditions
or attribute values.  */
 struct mapping {
   /* The name of the iterator or attribute.  */
@@ -50,82 +50,80 @@ struct mapping {
   /* The group (modes or codes) to which the iterator or attribute belongs.  */
   struct iterator_group *group;

-  /* Gives a unique number to the attribute or iterator.  Numbers are
- allocated consecutively, starting at 0.  */
-  int index;
-
   /* The list of (integer, string) pairs.  */
   struct map_value *values;
+
+  /* For iterators, records the current value of the iterator.  */
+  struct map_value *current_value;
 };

-/* A structure for abstracting the common parts of code and mode iterators.  */
+/* Vector definitions for the above.  */
+typedef struct mapping *mapping_ptr;
+DEF_VEC_P (mapping_ptr);
+DEF_VEC_ALLOC_P (mapping_ptr, heap);
+
+/* A structure for abstracting the common parts of iterators.  */
 struct iterator_group {
-  /* Tables of mapping structures, one for attributes and one for iterators. 
 */
+  /* Tables of mapping structures, one for attributes and one for
+ iterators.  */
   htab_t attrs, iterators;

-  /* The number of real modes or codes (and by extension, the first
- number available for use as an iterator placeholder).  */
-  int num_builtins;
-
-  /* Treat the given string as the name of a standard mode or code and
+  /* Treat the given string as the name of a standard mode, etc., and
  return its integer value.  */
   int (*find_builtin) (const char *);

-  /* Return true if the given rtx uses the given mode or code.  */
-  bool (*uses_iterator_p) (rtx, int);
+  /* Make the given pointer use the given iterator value.  */
+  void