diff mbox series

rtl: Enable the use of rtx values with int and mode attributes

Message ID b6e1ef17-b31b-4c8d-b62f-c1971d9da0fb@arm.com
State New
Headers show
Series rtl: Enable the use of rtx values with int and mode attributes | expand

Commit Message

Andre Vieira (lists) Aug. 13, 2024, 4:39 p.m. UTC
Hi,

The 'code' part of a 'define_code_attr' refers to the type of the key, 
in other words, it uses a code_iterator to pick the value from their 
(key "value") pair list.
Though it seems rtx_alloc_for_name requires a code_attribute to be used 
when the 'value' needs to be a type. In other words, no other type of 
attributes can be used to produce a rtx typed 'value'.

This patch removes that restriction and allows the backend to use any 
kind of attribute as long as that attribute always produces a valid code 
typed 'value'.

I also made some local, nonsense changes to test whether other 
attributes could be used regardless of type, for instance use an int 
attribute to produce modes:
+(define_int_iterator TEST_ITERATOR [UNSPEC_CALLEE_ABI UNSPEC_ABS])
+(define_int_attr test_attr [(UNSPEC_CALLEE_ABI "DI") (UNSPEC_ABS "SI")])
+
  (define_insn "*sibcall_insn"
    [(call (mem:DI (match_operand:DI 0 "aarch64_call_insn_operand" "Ucs, 
Usf")) (match_operand 1 ""))
-   (unspec:DI [(match_operand:DI 2 "const_int_operand")] UNSPEC_CALLEE_ABI)
+   (unspec:<test_attr> [(match_operand:<test_attr> 2 
"const_int_operand")] TEST_ITERATOR)
     (return)]

or a mode attribute to produce ints:

+(define_mode_attr test_mode_attr [(HF "0") (SF "0") (DF "0")])
+
  (define_split
    [(set (match_operand:GPF_HF 0 "nonimmediate_operand")
         (match_operand:GPF_HF 1 "const_double_operand"))]
    "can_create_pseudo_p ()
     && !aarch64_can_const_movi_rtx_p (operands[1], <MODE>mode)
     && !aarch64_float_const_representable_p (operands[1])
     && !aarch64_float_const_zero_rtx_p (operands[1])
     &&  aarch64_float_const_rtx_p (operands[1])"
-  [(const_int 0)]
+  [(const_int <test_mode_attr>)]

These all build fine.  Which makes me think this change would be 
consistent with other attributes.

Bootstrapped and regression tested aarch64-linux-gnu and x86_64-linux-gnu.

OK for trunk?

gcc/ChangeLog:

     * read-rtl.cc (rtx_reader::rtx_alloc_for_name): Allow all attribute
     types to produce code 'values'.
     (check_code_attribute): Rename ...
     (check_attribute_codes): ... to this.  And change comments to refer to
     any type of iterator.

Comments

Richard Sandiford Aug. 13, 2024, 5:06 p.m. UTC | #1
"Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com> writes:
> Hi,
>
> The 'code' part of a 'define_code_attr' refers to the type of the key, 
> in other words, it uses a code_iterator to pick the value from their 
> (key "value") pair list.
> Though it seems rtx_alloc_for_name requires a code_attribute to be used 
> when the 'value' needs to be a type. In other words, no other type of 
> attributes can be used to produce a rtx typed 'value'.
>
> This patch removes that restriction and allows the backend to use any 
> kind of attribute as long as that attribute always produces a valid code 
> typed 'value'.
>
> I also made some local, nonsense changes to test whether other 
> attributes could be used regardless of type, for instance use an int 
> attribute to produce modes:
> +(define_int_iterator TEST_ITERATOR [UNSPEC_CALLEE_ABI UNSPEC_ABS])
> +(define_int_attr test_attr [(UNSPEC_CALLEE_ABI "DI") (UNSPEC_ABS "SI")])
> +
>   (define_insn "*sibcall_insn"
>     [(call (mem:DI (match_operand:DI 0 "aarch64_call_insn_operand" "Ucs, 
> Usf")) (match_operand 1 ""))
> -   (unspec:DI [(match_operand:DI 2 "const_int_operand")] UNSPEC_CALLEE_ABI)
> +   (unspec:<test_attr> [(match_operand:<test_attr> 2 
> "const_int_operand")] TEST_ITERATOR)
>      (return)]
>
> or a mode attribute to produce ints:
>
> +(define_mode_attr test_mode_attr [(HF "0") (SF "0") (DF "0")])
> +
>   (define_split
>     [(set (match_operand:GPF_HF 0 "nonimmediate_operand")
>          (match_operand:GPF_HF 1 "const_double_operand"))]
>     "can_create_pseudo_p ()
>      && !aarch64_can_const_movi_rtx_p (operands[1], <MODE>mode)
>      && !aarch64_float_const_representable_p (operands[1])
>      && !aarch64_float_const_zero_rtx_p (operands[1])
>      &&  aarch64_float_const_rtx_p (operands[1])"
> -  [(const_int 0)]
> +  [(const_int <test_mode_attr>)]
>
> These all build fine.  Which makes me think this change would be 
> consistent with other attributes.
>
> Bootstrapped and regression tested aarch64-linux-gnu and x86_64-linux-gnu.
>
> OK for trunk?
>
> gcc/ChangeLog:
>
>      * read-rtl.cc (rtx_reader::rtx_alloc_for_name): Allow all attribute
>      types to produce code 'values'.
>      (check_code_attribute): Rename ...
>      (check_attribute_codes): ... to this.  And change comments to refer to
>      any type of iterator.
>
> diff --git a/gcc/read-rtl.cc b/gcc/read-rtl.cc
> index 4f09e449c81fb3d05bc566e6b9fb1787f4b3e31b..a6d0d41535d5db41bd3110c03a970cb451ea0706 100644
> --- a/gcc/read-rtl.cc
> +++ b/gcc/read-rtl.cc
> @@ -1423,21 +1423,21 @@ check_code_iterator (struct mapping *iterator)
>     consistent format.  Return a representative code.  */
>  
>  static rtx_code
> -check_code_attribute (mapping *attr)
> +check_attribute_codes (mapping *attr)
>  {
>    rtx_code bellwether = UNKNOWN;
>    for (map_value *v = attr->values; v != 0; v = v->next)
>      {
>        rtx_code code = maybe_find_code (v->string);
>        if (code == UNKNOWN)
> -	fatal_with_file_and_line ("code attribute `%s' contains "
> +	fatal_with_file_and_line ("attribute `%s' contains "
>  				  "unrecognized rtx code `%s'",
>  				  attr->name, v->string);
>        if (bellwether == UNKNOWN)
>  	bellwether = code;
>        else if (strcmp (GET_RTX_FORMAT (bellwether),
>  		       GET_RTX_FORMAT (code)) != 0)
> -	fatal_with_file_and_line ("code attribute `%s' combines "
> +	fatal_with_file_and_line ("attribute `%s' combines "
>  				  "`%s' and `%s', which have different "
>  				  "rtx formats", attr->name,
>  				  GET_RTX_NAME (bellwether),
> @@ -1604,7 +1604,7 @@ parse_reg_note_name (const char *string)
>    fatal_with_file_and_line ("unrecognized REG_NOTE name: `%s'", string);
>  }
>  
> -/* Allocate an rtx for code NAME.  If NAME is a code iterator or code
> +/* Allocate an rtx for code NAME.  If NAME is a code iterator or an
>     attribute, record its use for later and use one of its possible
>     values as an interim rtx code.  */
>  
> @@ -1629,11 +1629,15 @@ rtx_reader::rtx_alloc_for_name (const char *name)
>        /* Find the attribute itself.  */
>        mapping *m = (mapping *) htab_find (codes.attrs, &attr);
>        if (!m)
> -	fatal_with_file_and_line ("unknown code attribute `%s'", attr);
> +	m = (mapping *) htab_find (ints.attrs, &attr);
> +      if (!m)
> +	m = (mapping *) htab_find (modes.attrs, &attr);
> +      if (!m)
> +	fatal_with_file_and_line ("unknown attribute `%s'", attr);

Could you chnage this to:

      mapping *m = nullptr;
      for (auto attrs : { codes.attrs, ints.attrs, modes.attrs })
	if (auto *newm = (mapping *) htab_find (attrs, &attr))
	  {
	    if (m)
	      fatal_with_file_and_line ("ambiguous attribute `%s'", attr);
	    m = newm;
	  }
      if (!m)
	fatal_with_file_and_line ("unknown attribute `%s'", attr);

so that we flag any potential conflict?  The later ambiguity detection
only applies to iterators that are actually used, and so there might be
no ambiguity at that stage.  But it's possible that this code and that
code could pick different attributes.

The documentation currently reads:

-------------------------------------------------------------------------
Instruction patterns can use code attributes as rtx codes, which can be
useful if two sets of codes act in tandem.  For example, the following
@code{define_insn} defines two patterns, one calculating a signed absolute
difference and another calculating an unsigned absolute difference:

@smallexample
(define_code_iterator any_max [smax umax])
(define_code_attr paired_min [(smax "smin") (umax "umin")])
(define_insn @dots{}
  [(set (match_operand:SI 0 @dots{})
        (minus:SI (any_max:SI (match_operand:SI 1 @dots{})
                              (match_operand:SI 2 @dots{}))
                  (<paired_min>:SI (match_dup 1) (match_dup 2))))]
  @dots{})
@end smallexample

The signed version of the instruction uses @code{smax} and @code{smin}
while the unsigned version uses @code{umax} and @code{umin}.  There
are no versions that pair @code{smax} with @code{umin} or @code{umax}
with @code{smin}.
-------------------------------------------------------------------------

Maybe it would be enough add an additional paragraph:

-------------------------------------------------------------------------
It is also possible to use other types of attributes as codes,
in a similar way.  For example, an int iterator could be used to
iterate over @code{unspec} numbers, with an int attribute specifying
an associated rtx code.  @xref{Int Iterators}.
-------------------------------------------------------------------------

OK with those changes, thanks.

Richard

>  
>        /* Pick the first possible code for now, and record the attribute
>  	 use for later.  */
> -      rtx x = rtx_alloc (check_code_attribute (m));
> +      rtx x = rtx_alloc (check_attribute_codes (m));
>        record_attribute_use (&codes, get_current_location (),
>  			    x, 0, deferred_name);
>        return x;
diff mbox series

Patch

diff --git a/gcc/read-rtl.cc b/gcc/read-rtl.cc
index 4f09e449c81fb3d05bc566e6b9fb1787f4b3e31b..a6d0d41535d5db41bd3110c03a970cb451ea0706 100644
--- a/gcc/read-rtl.cc
+++ b/gcc/read-rtl.cc
@@ -1423,21 +1423,21 @@  check_code_iterator (struct mapping *iterator)
    consistent format.  Return a representative code.  */
 
 static rtx_code
-check_code_attribute (mapping *attr)
+check_attribute_codes (mapping *attr)
 {
   rtx_code bellwether = UNKNOWN;
   for (map_value *v = attr->values; v != 0; v = v->next)
     {
       rtx_code code = maybe_find_code (v->string);
       if (code == UNKNOWN)
-	fatal_with_file_and_line ("code attribute `%s' contains "
+	fatal_with_file_and_line ("attribute `%s' contains "
 				  "unrecognized rtx code `%s'",
 				  attr->name, v->string);
       if (bellwether == UNKNOWN)
 	bellwether = code;
       else if (strcmp (GET_RTX_FORMAT (bellwether),
 		       GET_RTX_FORMAT (code)) != 0)
-	fatal_with_file_and_line ("code attribute `%s' combines "
+	fatal_with_file_and_line ("attribute `%s' combines "
 				  "`%s' and `%s', which have different "
 				  "rtx formats", attr->name,
 				  GET_RTX_NAME (bellwether),
@@ -1604,7 +1604,7 @@  parse_reg_note_name (const char *string)
   fatal_with_file_and_line ("unrecognized REG_NOTE name: `%s'", string);
 }
 
-/* Allocate an rtx for code NAME.  If NAME is a code iterator or code
+/* Allocate an rtx for code NAME.  If NAME is a code iterator or an
    attribute, record its use for later and use one of its possible
    values as an interim rtx code.  */
 
@@ -1629,11 +1629,15 @@  rtx_reader::rtx_alloc_for_name (const char *name)
       /* Find the attribute itself.  */
       mapping *m = (mapping *) htab_find (codes.attrs, &attr);
       if (!m)
-	fatal_with_file_and_line ("unknown code attribute `%s'", attr);
+	m = (mapping *) htab_find (ints.attrs, &attr);
+      if (!m)
+	m = (mapping *) htab_find (modes.attrs, &attr);
+      if (!m)
+	fatal_with_file_and_line ("unknown attribute `%s'", attr);
 
       /* Pick the first possible code for now, and record the attribute
 	 use for later.  */
-      rtx x = rtx_alloc (check_code_attribute (m));
+      rtx x = rtx_alloc (check_attribute_codes (m));
       record_attribute_use (&codes, get_current_location (),
 			    x, 0, deferred_name);
       return x;