Patchwork RFA: Alternative iterator implementation

login
register
mail settings
Submitter Tejas Belagod
Date June 12, 2012, 4:21 p.m.
Message ID <4FD76C9F.9040904@arm.com>
Download mbox | patch
Permalink /patch/164446/
State New
Headers show

Comments

Tejas Belagod - June 12, 2012, 4:21 p.m.
Richard Sandiford wrote:
> Thanks for the update.
> 
> Tejas Belagod <tbelagod@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.
Richard Sandiford - June 12, 2012, 6:26 p.m.
Tejas Belagod <tbelagod@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
Richard Henderson - June 12, 2012, 9:29 p.m.
On 2012-06-12 11:26, Richard Sandiford wrote:
> Tejas Belagod <tbelagod@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~

Patch

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_vq<absneg><mode>"
+  [(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"
+  "vq<absneg>.<V_s_elem>\t%<V_reg>0, %<V_reg>1"
+  [(set_attr "neon_type" "neon_vqneg_vqabs")]
+)
+
+@end smallexample
+
+This is equivalent to:
+
+@smallexample
+(define_insn "neon_vqabs<mode>"
+  [(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_reg>0, %<V_reg>1"
+  [(set_attr "neon_type" "neon_vqneg_vqabs")]
+)
+
+(define_insn "neon_vqneg<mode>"
+  [(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_VQNEG))]
+  "TARGET_NEON"
+  "vqneg.<V_s_elem>\t%<V_reg>0, %<V_reg>1"
+  [(set_attr "neon_type" "neon_vqneg_vqabs")]
+)
+
+@end smallexample
+
 @end ifset
diff --git a/gcc/read-rtl.c b/gcc/read-rtl.c
index c588722..71ecf53 100644
--- a/gcc/read-rtl.c
+++ b/gcc/read-rtl.c
@@ -114,7 +114,7 @@  static rtx read_nested_rtx (void);
 static rtx read_rtx_variadic (rtx);
 
 /* The mode and code iterator structures.  */
-static struct iterator_group modes, codes;
+static struct iterator_group modes, codes, ints;
 
 /* All iterators used in the current rtx.  */
 static VEC (mapping_ptr, heap) *current_iterators;
@@ -165,6 +165,25 @@  apply_code_iterator (void *loc, int code)
   PUT_CODE ((rtx) loc, (enum rtx_code) code);
 }
 
+/* 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)
+{
+  validate_const_int (name);
+  return atoi (name);
+}
+
+static void
+apply_int_iterator (void *loc, int value)
+{
+  *(int *)loc = value;
+}
+
 /* Map attribute string P to its current value.  Return null if the attribute
    isn't known.  */
 
@@ -412,6 +431,7 @@  apply_iterators (rtx original, rtx *queue)
      definition order within each group.  */
   htab_traverse (modes.iterators, add_current_iterators, NULL);
   htab_traverse (codes.iterators, add_current_iterators, NULL);
+  htab_traverse (ints.iterators, add_current_iterators, NULL);
   gcc_assert (!VEC_empty (mapping_ptr, current_iterators));
 
   for (;;)
@@ -518,6 +538,12 @@  initialize_iterators (void)
   codes.find_builtin = find_code;
   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.find_builtin = find_int;
+  ints.apply_iterator = apply_int_iterator;
+
   lower = add_mapping (&modes, modes.attrs, "mode");
   upper = add_mapping (&modes, modes.attrs, "MODE");
   lower_ptr = &lower->values;
@@ -827,6 +853,16 @@  read_rtx (const char *rtx_name, rtx *x)
       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;
+    }
 
   apply_iterators (read_rtx_code (rtx_name), &queue_head);
   VEC_truncate (iterator_use, iterator_uses, 0);
@@ -850,7 +886,6 @@  read_rtx_code (const char *code_name)
   struct md_name name;
   rtx return_rtx;
   int c;
-  int tmp_int;
   HOST_WIDE_INT tmp_wide;
 
   /* Linked list structure for making RTXs: */
@@ -1026,10 +1061,10 @@  read_rtx_code (const char *code_name)
 
       case 'i':
       case 'n':
+	/* 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;
+	record_potential_iterator_use (&ints, &XINT (return_rtx, i),
+				       name.string);
 	break;
 
       default: