"Andre Vieira (lists)" <andre.simoesdiasvie...@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;