From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id EEEC73858C62 for ; Thu, 8 Jun 2023 14:24:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EEEC73858C62 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0C91DAB6; Thu, 8 Jun 2023 07:25:13 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.110.72]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B26E73F6C4; Thu, 8 Jun 2023 07:24:26 -0700 (PDT) From: Richard Sandiford To: Tamar Christina Mail-Followup-To: Tamar Christina ,Richard Earnshaw , "gcc-patches\@gcc.gnu.org" , nd , richard.sandiford@arm.com Cc: Richard Earnshaw , "gcc-patches\@gcc.gnu.org" , nd Subject: Re: [PATCH v2] machine descriptor: New compact syntax for insn and insn_split in Machine Descriptions. References: Date: Thu, 08 Jun 2023 15:24:25 +0100 In-Reply-To: (Tamar Christina's message of "Thu, 8 Jun 2023 10:58:50 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-27.8 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KAM_LOTSOFHASH,KAM_SHORT,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: In addition to Andreas's and Richard's comments: Tamar Christina writes: > +@item > +@samp{@{@@} is followed by a layout in parentheses which is @samp{cons:}= followed by > +a list of @code{match_operand}/@code{match_scratch} comma operand number= s, then a How about: a comma-separated list of @code{match_operand}/@code{match_scratch} opera= nd numbers, then a Some lines are >80 chars. > +semicolon, followed by the same for attributes (@samp{attrs:}). Operand > +modifiers can be placed in this section group as well. Both sections > +are optional (so you can use only @samp{cons}, or only @samp{attrs}, or = both), > +and @samp{cons} must come before @samp{attrs} if present. > + > +@item > +Each alternative begins with any amount of whitespace. > + > +@item > +Following the whitespace is a comma-separated list of "constraints" and/= or > +"attributes" within brackets @code{[]}, with sections separated by a sem= icolon. > + > +@item > +Should you want to copy the previous asm line, the symbol @code{^} can b= e used. > +This allows less copy pasting between alternative and reduces the number= of > +lines to update on changes. > + > +@item > +When using C functions for output, the idiom @samp{* return ;}= can be @samp{* return @var{function};} > +replaced with the shorthand @samp{<< @var{function};}. > + > +@item > +Following the closing @samp{]} is any amount of whitespace, and then the= actual > +asm output. > + > +@item > +Spaces are allowed in the list (they will simply be removed). > + > +@item > +All constraint alternatives should be specified: a blank list should be > +@samp{[,,]} or generally use @samp{*} for the alternatives. e.g. @samp{[= *,*,*]}. I think this is mixing two things. How about: @item All constraint alternatives should be specified. For example, a list of of three blank alternatives should be written @samp{[,,]} rather than @samp{[]}. @item All attribute alternatives should be non-empty, with @samp{*} representing the default attribute value. For example, a list of three default attribute values should be written @samp{[*,*,*]} rather than @samp{[]}. > + > +@item > +Within an @samp{@{@@} block both multiline and singleline C comments are > +allowed, but when used outside of a C block they must be the only non-wh= itespace > +blocks on the line. > + > +@item > +Within an @samp{@{@@} block, any iterators that do not get expanded will= result in an > +error. If for some reason it is required to have @code{<>} in the outpu= t then Maybe better as: s/@code{<>}/@code{<} or @code{>}/ > +these must be escaped using @backslashchar{}. > + > +@item > +The actual constraint string in the @code{match_operand} or > +@code{match_scratch}, and the attribute string in the @code{set_attr}, m= ust be > +blank or an empty string (you can't combine the old and new syntaxes). It looks like the new version drops support for the set_attr case though (thanks). > + > +@item > +Additional @code{set_attr} can be specified other than the ones in the > +@samp{attrs} list. These must use the normal syntax and must come last.= There > +must not be any overlap between the two lists. Similarly here: I don't think the =E2=80=9Cthey must come last=E2=80=9D bit= applies any more. How about something like: It is possible to use the @samp{attrs} list to specify some attributes and to use the normal @code{set_attr} syntax to specify other attributes. There must not be any overlap between the two lists. > + > +In other words, the following is valid: > +@smallexample > +@group > +(define_insn_and_split "" > + [(set (match_operand:SI 0 "nonimmediate_operand") > + (match_operand:SI 1 "aarch64_mov_operand"))] > + "" > + @{@@ [cons: 0, 1; attrs: type, arch, length]@} > + @dots{} > + [(set_attr "foo" "mov_imm")] > +) > +@end group > +@end smallexample > + > +but these are not valid: > +@smallexample > +@group > +(define_insn_and_split "" > + [(set (match_operand:SI 0 "nonimmediate_operand") > + (match_operand:SI 1 "aarch64_mov_operand"))] > + "" > + @{@@ [cons: 0, 1; attrs: type, arch, length]@} > + @dots{} > + [(set_attr "type") > + (set_attr "arch") > + (set_attr "foo" "mov_imm")] > +) > +@end group > +@end smallexample > + > +and > + > +@smallexample > +@group > +(define_insn_and_split "" > + [(set (match_operand:SI 0 "nonimmediate_operand") > + (match_operand:SI 1 "aarch64_mov_operand"))] > + "" > + @{@@ [cons: 0, 1; attrs: type, arch, length]@} > + @dots{} > + [(set_attr "type") > + (set_attr "foo" "mov_imm") > + (set_attr "arch") > + (set_attr "length")] > +) > +@end group > +@end smallexample > + > +because the order of the entries don't match and new entries must be las= t. > +@end itemize These examples probably need updating too. > + > @node Predicates > @section Predicates > @cindex predicates > diff --git a/gcc/genoutput.cc b/gcc/genoutput.cc > index 163e8dfef4ca2c2c92ce1cf001ee6be40a54ca3e..7088f816cfa6e6ab2c1f51b8b= baa5eae990a0a4b 100644 > --- a/gcc/genoutput.cc > +++ b/gcc/genoutput.cc > @@ -157,6 +157,7 @@ public: > int n_alternatives; /* Number of alternatives in each constra= int */ > int operand_number; /* Operand index in the big array. */ > int output_format; /* INSN_OUTPUT_FORMAT_*. */ > + bool compact_syntax_p; > struct operand_data operand[MAX_MAX_OPERANDS]; > }; > > @@ -700,12 +701,51 @@ process_template (class data *d, const char *templa= te_code) > if (sp !=3D ep) > message_at (d->loc, "trailing whitespace in output template"); > > - while (cp < sp) > + /* Check for any unexpanded iterators. */ > + if (bp[0] !=3D '*' && d->compact_syntax_p) > { > - putchar (*cp); > - cp++; > + const char *p =3D cp; > + const char *last_bracket =3D nullptr; > + while (p < sp) > + { > + if (*p =3D=3D '\\' && p + 1 < sp) > + { > + putchar (*p); > + putchar (*(p+1)); > + p +=3D 2; > + continue; > + } > + > + if (*p =3D=3D '>' && last_bracket && *last_bracket =3D= =3D '<') > + { > + int len =3D p - last_bracket; > + fatal_at (d->loc, "unresolved iterator '%.*s' in '%= s'", > + len - 1, last_bracket + 1, cp); > + } > + else if (*p =3D=3D '<' || *p =3D=3D '>') > + last_bracket =3D p; > + > + putchar (*p); > + p +=3D 1; > + } > + > + if (last_bracket) > + { > + char *nl =3D strchr (const_cast (cp), '\n'); > + if (nl) > + *nl =3D'\0'; Nit: missing space after "=3D". > + fatal_at (d->loc, "unmatched angle brackets, likely an " > + "error in iterator syntax in %s", cp); > + } > + } > + else > + { > + while (cp < sp) > + putchar (*(cp++)); > } > > + cp =3D sp; > + > if (!found_star) > puts ("\","); > else if (*bp !=3D '*') > @@ -881,6 +921,8 @@ gen_insn (md_rtx_info *info) > else > d->name =3D 0; > > + d->compact_syntax_p =3D compact_syntax.contains (insn); > + > /* Build up the list in the same order as the insns are seen > in the machine description. */ > d->next =3D 0; > diff --git a/gcc/gensupport.h b/gcc/gensupport.h > index a1edfbd71908b6244b40f801c6c01074de56777e..7925e22ed418767576567cad5= 83bddf83c0846b1 100644 > --- a/gcc/gensupport.h > +++ b/gcc/gensupport.h > @@ -20,6 +20,7 @@ along with GCC; see the file COPYING3. If not see > #ifndef GCC_GENSUPPORT_H > #define GCC_GENSUPPORT_H > > +#include "hash-set.h" > #include "read-md.h" > > struct obstack; > @@ -218,6 +219,8 @@ struct pattern_stats > int num_operand_vars; > }; > > +extern hash_set compact_syntax; > + > extern void get_pattern_stats (struct pattern_stats *ranges, rtvec vec); > extern void compute_test_codes (rtx, file_location, char *); > extern file_location get_file_location (rtx); > diff --git a/gcc/gensupport.cc b/gcc/gensupport.cc > index f9efc6eb7572a44b8bb154b0b22be3815bd0d244..507285e8fef5443ec42501447= e730022e1426ee1 100644 > --- a/gcc/gensupport.cc > +++ b/gcc/gensupport.cc > @@ -18,6 +18,8 @@ > . */ > > #include "bconfig.h" > +#define INCLUDE_STRING > +#define INCLUDE_VECTOR > #include "system.h" > #include "coretypes.h" > #include "tm.h" > @@ -33,6 +35,8 @@ > static rtx operand_data[MAX_OPERANDS]; > static rtx match_operand_entries_in_pattern[MAX_OPERANDS]; > static char used_operands_numbers[MAX_OPERANDS]; > +/* List of entries which are part of the new syntax. */ > +hash_set compact_syntax; > > > /* In case some macros used by files we include need it, define this her= e. */ > @@ -545,6 +549,526 @@ gen_rewrite_sequence (rtvec vec) > return new_vec; > } > > +/* The following is for handling the compact syntax for constraints and > + attributes. > + > + The normal syntax looks like this: > + > + ... > + (match_operand: 0 "s_register_operand" "r,I,k") > + (match_operand: 2 "s_register_operand" "r,k,I") > + ... > + "@ > + > + > + " > + ... > + (set_attr "length" "4,8,8") > + > + The compact syntax looks like this: > + > + ... > + (match_operand: 0 "s_register_operand") > + (match_operand: 2 "s_register_operand") > + ... > + {@ [cons: 0, 2; attrs: length] > + [r,r; 4] > + [I,k; 8] > + [k,I; 8] > + } > + ... > + (set_attr "length") Similarly here, I think the final set_attr is no longer expected. > + > + This is the only place where this syntax needs to be handled. Releva= nt > + patterns are transformed from compact to the normal syntax before the= y are > + queued, so none of the gen* programs need to know about this syntax a= t all. > + > + Conversion process (convert_syntax): > + > + 0) Check that pattern actually uses new syntax (check for {@ ... }). > + > + 1) Get the "layout", i.e. the "[cons: 0 2; attrs: length]" from the a= bove > + example. cons must come first; both are optional. Set up two vecs, > + convec and attrvec, for holding the results of the transformation. > + > + 2) For each alternative: parse the list of constraints and/or attribu= tes, > + and enqueue them in the relevant lists in convec and attrvec. By = the end > + of this process, convec[N].con and attrvec[N].con should contain r= egular > + syntax constraint/attribute lists like "r,I,k". Copy the asm to a= string > + as we go. > + > + 3) Search the rtx and write the constraint and attribute lists into t= he > + correct places. Write the asm back into the template. */ > + > +/* Helper class for shuffling constraints/attributes in convert_syntax a= nd > + add_constraints/add_attributes. This includes commas but not whitesp= ace. */ > + > +class conlist { > +private: > + std::string con; > + > +public: > + std::string name; > + int idx =3D -1; > + > + conlist () =3D default; > + > + /* [ns..ns + len) should be a string with the id of the rtx to match > + i.e. if rtx is the relevant match_operand or match_scratch then > + [ns..ns + len) should equal itoa (XINT (rtx, 0)), and if set_attr t= hen > + [ns..ns + len) should equal XSTR (rtx, 0). */ > + conlist (const char *ns, unsigned int len, bool numeric) > + { > + /* Trim leading whitespaces. */ > + while (ISSPACE (*ns)) > + { > + ns++; > + len--; > + } > + > + /* Trim trailing whitespace. */ > + for (int i =3D len - 1; i >=3D 0; i++, len--) > + if (!ISSPACE (*ns)) > + break; As you pointed out off-list, these should be ISBLANK rather than ISSPACE. Sorry for missing the effect on '\n'. > + /* Parse off any modifiers. */ > + while (!ISALNUM (*ns)) > + { > + con +=3D *(ns++); > + len--; > + } > + > + name.assign (ns, len); > + if (numeric) > + idx =3D std::stoi (name); > + } > + > + /* Adds a character to the end of the string. */ > + void add (char c) > + { > + con +=3D c; > + } > + > + /* Output the string in the form of a brand-new char *, then effective= ly > + clear the internal string by resetting len to 0. */ > + char *out () > + { > + /* Final character is always a trailing comma, so strip it out. */ > + char *q =3D xstrndup (con.c_str (), con.size () - 1); > + con.clear (); > + return q; > + } > +}; > + > +typedef std::vector vec_conlist; > + > +/* Add constraints to an rtx. This function is similar to remove_constr= aints. > + Errors if adding the constraints would overwrite existing constraints= . */ > + > +static void > +add_constraints (rtx part, file_location loc, vec_conlist &cons) > +{ > + const char *format_ptr; > + > + if (part =3D=3D NULL_RTX) > + return; > + > + /* If match_op or match_scr, check if we have the right one, and if so= , copy > + over the constraint list. */ > + if (GET_CODE (part) =3D=3D MATCH_OPERAND || GET_CODE (part) =3D=3D MAT= CH_SCRATCH) > + { > + int field =3D GET_CODE (part) =3D=3D MATCH_OPERAND ? 2 : 1; > + unsigned id =3D XINT (part, 0); > + > + if (id >=3D cons.size ()) > + fatal_at (loc, "could not find match_operand/scratch with id %d",= id); > + > + if (cons[id].idx =3D=3D -1) > + { > + error_at (loc, "constructor %d encountered more than once", id); > + return; > + } -1 can also mean that the cons: didn't list "id" at all. So if we want to detect duplicates, we would need to assign something else (such as -2) when consuming a constraint. But I don't think it's necessary to check for duplicates here, since later code should do that for both syntaxes. So there are two options: (1) Return early without error if "cons[id].idx =3D=3D -1". Continue to use "cons[id].idx =3D -1" when consuming the constraint. =20=20=20=20 (2) Return early without error if "cons[id].idx =3D=3D -1". Flag an error if "cons[id].idx =3D=3D -2". Use "cons[id].idx =3D -2" when consuming the constraint. I was thinking of (1), but (2) would also be OK if you prefer. > + if (XSTR (part, field)[0] !=3D '\0') > + { > + error_at (loc, "can't mix normal and compact constraint syntax"= ); > + return; > + } > + XSTR (part, field) =3D cons[id].out (); > + cons[id].idx =3D -1; > + } > + > + format_ptr =3D GET_RTX_FORMAT (GET_CODE (part)); > + > + /* Recursively search the rtx. */ > + for (int i =3D 0; i < GET_RTX_LENGTH (GET_CODE (part)); i++) > + switch (*format_ptr++) > + { > + case 'e': > + case 'u': > + add_constraints (XEXP (part, i), loc, cons); > + break; > + case 'E': > + if (XVEC (part, i) !=3D NULL) > + for (int j =3D 0; j < XVECLEN (part, i); j++) > + add_constraints (XVECEXP (part, i, j), loc, cons); > + break; > + default: > + continue; > + } > +} > + > +/* Add attributes to an rtx. The attributes that are matched must be in = order > + i.e. read from top to bottom in the pattern. > + Errors if adding the attributes would overwrite existing attributes. > + Returns 1 + index of last conlist to be matched. */ > + > +static unsigned int > +add_attributes (rtx x, file_location loc, vec_conlist &attrs) > +{ > + unsigned int attr_index =3D GET_CODE (x) =3D=3D DEFINE_INSN ? 4 : 3; > + unsigned int index =3D 0; > + > + if (XVEC (x, attr_index) =3D=3D NULL) > + return index; > + > + for (int i =3D 0; i < XVECLEN (x, attr_index); ++i) > + { > + rtx part =3D XVECEXP (x, attr_index, i); > + > + if (GET_CODE (part) !=3D SET_ATTR) > + continue; > + > + if (attrs[index].name =3D=3D XSTR (part, 0)) > + { > + if (XSTR (part, 1) && XSTR (part, 1)[0] !=3D '\0') > + fatal_at (loc, "can't mix normal and compact attribute syntax= "); > + XSTR (part, 1) =3D attrs[index].out (); > + > + if (++index =3D=3D attrs.size ()) > + break; > + } > + else > + fatal_at (loc, "can't mix normal and compact attribute syntax"); > + } > + > + return index; > +} > + > +/* Modify the attributes list to make space for the implicitly declared > + attributes in the attrs: list. */ > + > +static void > +create_missing_attributes (rtx x, file_location /* loc */, vec_conlist &= attrs) > +{ > + if (attrs.empty ()) > + return; > + > + unsigned int attr_index =3D GET_CODE (x) =3D=3D DEFINE_INSN ? 4 : 3; > + rtvec orig =3D XVEC (x, attr_index); > + size_t n_curr =3D orig ? XVECLEN (x, attr_index) : 0; > + rtvec copy =3D rtvec_alloc (n_curr + attrs.size ()); > + > + /* Create a shallow copy of existing entries. */ > + memcpy (©->elem[attrs.size ()], &orig->elem[0], sizeof (rtx) * n_c= urr); > + XVEC (x, attr_index) =3D copy; > + > + /* Create the new elements. */ > + for (unsigned i =3D 0; i < attrs.size (); i++) > + { > + rtx attr =3D rtx_alloc (SET_ATTR); > + XSTR (attr, 0) =3D xstrdup (attrs[i].name.c_str ()); > + XSTR (attr, 1) =3D NULL; > + XVECEXP (x, attr_index, i) =3D attr; > + } > + > + return; > +} I think we should be able to combine the last two functions into something like: /* Add ATTRS to definition X's attribute list. */ static void add_attributes (rtx x, vec_conlist &attrs) { unsigned int attr_index =3D GET_CODE (x) =3D=3D DEFINE_INSN ? 4 : 3; rtvec orig =3D XVEC (x, attr_index); size_t n_curr =3D orig ? XVECLEN (x, attr_index) : 0; rtvec copy =3D rtvec_alloc (n_curr + attrs.size ()); /* Create a shallow copy of existing entries. */ memcpy (©->elem[attrs.size ()], &orig->elem[0], sizeof (rtx) * n_curr= ); XVEC (x, attr_index) =3D copy; /* Create the new elements. */ for (unsigned i =3D 0; i < attrs.size (); i++) { rtx attr =3D rtx_alloc (SET_ATTR); XSTR (attr, 0) =3D xstrdup (attrs[i].name.c_str ()); XSTR (attr, 1) =3D attrs[i].out (); XVECEXP (x, attr_index, i) =3D attr; } } Then drop the call to create_missing_attributes and replace: if (attrvec.size () > 0) { index =3D add_attributes (x, loc, attrvec); if (index < attrvec.size ()) fatal_at (loc, "could not find set_attr for attribute %s", attrvec[index].name.c_str ()); } with: if (attrvec.size () > 0) add_attributes (x, attrs); > + > +/* Consumes spaces and tabs. */ > + > +static inline void > +skip_spaces (const char **str) > +{ > + while (ISSPACE (**str)) ISBLANK here too (sorry). > + (*str)++; > +} > + > +/* Consumes the given character, if it's there. */ > + > +static inline bool > +expect_char (const char **str, char c) > +{ > + if (**str !=3D c) > + return false; > + (*str)++; > + return true; > +} > + > +/* Parses the section layout that follows a "{@}" if using new syntax. B= uilds "{@" > + a vector for a single section. E.g. if we have "attrs: length arch]..= ." "attrs: length, arch]..." > + then list will have two elements, the first for "length" and the seco= nd > + for "arch". */ > + > +static void > +parse_section_layout (const char **templ, const char *label, > + vec_conlist &list, bool numeric) > +{ > + const char *name_start; > + size_t label_len =3D strlen (label); > + if (strncmp (label, *templ, label_len) =3D=3D 0) > + { > + *templ +=3D label_len; > + > + /* Gather the names. */ > + while (**templ !=3D ';' && **templ !=3D ']') > + { > + skip_spaces (templ); > + name_start =3D *templ; > + int len =3D 0; > + char val =3D (*templ)[len]; > + while (val !=3D ',' && val !=3D ';' && val !=3D ']') > + val =3D (*templ)[++len]; Sorry for not noticing last time, but this loop should raise an error if val is 0 or '\n', before the assignment of a new val. Otherwise a malformed string would lead to a segfault. E.g. something like: while (val !=3D ',' && val !=3D ';' && val !=3D ']') { if (val =3D=3D 0 || val =3D=3D '\n') fatal_at (loc, "missing ']'"); val =3D (*templ)[++len]; } > + *templ +=3D len; > + if (val =3D=3D ',') > + (*templ)++; > + list.push_back (conlist (name_start, len, numeric)); > + } > + } > +} > + > +/* Parse a section, a section is defined as a named space separated list= , e.g. > + > + foo: a, b, c > + > + is a section named "foo" with entries a, b and c. */ > + > +static void > +parse_section (const char **templ, unsigned int n_elems, unsigned int al= t_no, > + vec_conlist &list, file_location loc, const char *name) > +{ > + unsigned int i; > + > + /* Go through the list, one character at a time, adding said character > + to the correct string. */ > + for (i =3D 0; **templ !=3D ']' && **templ !=3D ';'; (*templ)++) > + { > + if (!ISSPACE (**templ)) ISBLANK here too. No need for the "for" loop to have braces. > + { Here too we should check **templ for 0 or '\n', and raise an error if found. > + list[i].add(**templ); Nit: should be a space after "add". > + if (**templ =3D=3D ',') > + { > + ++i; > + if (i =3D=3D n_elems) > + fatal_at (loc, "too many %ss in alternative %d: expected = %d", > + name, alt_no, n_elems); > + } > + } > + } > + > + if (i + 1 < n_elems) > + fatal_at (loc, "too few %ss in alternative %d: expected %d, got %d", > + name, alt_no, n_elems, i); > + > + list[i].add(','); Nit: should be a space after "add". > +} > + > +/* The compact syntax has more convience syntaxes. As such we post proc= ess > + the lines to get them back to something the normal syntax understands= . */ > + > +static void > +preprocess_compact_syntax (file_location loc, int alt_no, std::string &l= ine, > + std::string &last_line) > +{ > + /* Check if we're copying the last statement. */ > + if (line.find ("^") =3D=3D 0 && line.size () =3D=3D 1) > + { > + if (last_line.empty ()) > + fatal_at (loc, "found instruction to copy previous line (^) in" > + "alternative %d but no previous line to copy", alt= _no); > + line =3D last_line; > + return; > + } > + > + std::string result; > + std::string buffer; > + /* Check if we have << which means return c statement. */ > + if (line.find ("<<") =3D=3D 0) > + { > + result.append ("* return "); > + const char *chunk =3D line.c_str () + 2; > + skip_spaces (&chunk); > + result.append (chunk); > + } > + else > + result.append (line); > + > + line =3D result; > + return; > +} > + > +/* Converts an rtx from compact syntax to normal syntax if possible. */ > + > +static void > +convert_syntax (rtx x, file_location loc) > +{ > + int alt_no; > + unsigned int index, templ_index; > + const char *templ; > + vec_conlist tconvec, convec, attrvec; > + > + templ_index =3D GET_CODE (x) =3D=3D DEFINE_INSN ? 3 : 2; > + > + templ =3D XTMPL (x, templ_index); > + > + /* Templates with constraints start with "{@". */ > + if (strncmp ("*{@", templ, 3)) > + return; > + > + /* Get the layout for the template. */ > + templ +=3D 3; > + skip_spaces (&templ); > + > + if (!expect_char (&templ, '[')) > + fatal_at (loc, "expecing `[' to begin section list"); > + > + parse_section_layout (&templ, "cons:", tconvec, true); > + convec.resize (tconvec.size ()); > + > + /* Check for any duplicate cons entries and sort based on i. */ > + for (auto e : tconvec) > + { > + unsigned idx =3D e.idx; > + if (idx >=3D convec.size ()) > + convec.resize (idx + 1); > + > + if (convec[idx].idx >=3D 0) > + fatal_at (loc, "duplicate cons number found: %d", idx); > + convec[idx] =3D e; > + } > + tconvec.clear (); > + > + if (*templ !=3D ']') > + { > + if (*templ =3D=3D ';') > + skip_spaces (&(++templ)); > + parse_section_layout (&templ, "attrs:", attrvec, false); > + create_missing_attributes (x, loc, attrvec); > + } > + > + if (!expect_char (&templ, ']')) > + fatal_at (loc, "expecting `]` to end section list - section list mus= t have " > + "cons first, attrs second"); > + > + /* We will write the un-constrainified template into new_templ. */ > + std::string new_templ; > + new_templ.append ("@"); > + > + /* Skip to the first proper line. */ > + skip_spaces (&templ); > + > + alt_no =3D 0; > + std::string last_line; > + > + /* Process the alternatives. */ > + while (*(templ - 1) !=3D '\0') > + { > + /* Skip leading whitespace. */ > + std::string buffer; > + skip_spaces (&templ); > + > + /* Check if we're at the end. */ > + if (templ[0] =3D=3D '}' && templ[1] =3D=3D '\0') > + break; > + > + new_templ +=3D '\n'; > + new_templ.append (buffer); Shouldn't this be... > + > + if (expect_char (&templ, '[')) > + { ...here, to prevent an extra '\n' from being added if the template ends with a comment? > + /* Parse the constraint list, then the attribute list. */ > + if (convec.size () > 0) > + parse_section (&templ, convec.size (), alt_no, convec, loc, > + "constraint"); > + > + if (attrvec.size () > 0) > + { > + if (convec.size () > 0 && !expect_char (&templ, ';')) > + fatal_at (loc, "expected `;' to separate constraints " > + "and attributes in alternative %d", alt_no= ); > + > + parse_section (&templ, attrvec.size (), alt_no, > + attrvec, loc, "attribute"); > + } > + > + if (!expect_char (&templ, ']')) > + fatal_at (loc, "expected end of constraint/attribute list but= " > + "missing an ending `]' in alternative %d", alt= _no); > + } > + else if (templ[0] =3D=3D '/' && templ[1] =3D=3D '/') > + { > + templ +=3D 2; > + /* Glob till newline or end of string. */ > + while (*templ !=3D '\n' || *templ !=3D '\0') > + templ++; > + templ++; > + continue; > + } > + else if (templ[0] =3D=3D '/' && templ[1] =3D=3D '*') > + { > + templ +=3D 2; > + /* Glob till newline or end of multiline comment. */ > + while (templ[0] !=3D '*' && templ[1] !=3D '/') > + templ++; This loop should check for templ[0] =3D=3D 0 too. > + templ +=3D 2; > + continue; > + } > + else > + fatal_at (loc, "expected constraint/attribute list at beginning o= f " > + "alternative %d but missing a starting `['", alt_n= o); > + > + /* Skip whitespace between list and asm. */ > + ++templ; > + skip_spaces (&templ); > + > + /* Copy asm to new template. */ > + std::string line; > + while (*templ !=3D '\n' && *templ !=3D '\0') > + line +=3D *templ++; > + > + /* Apply any pre-processing needed to the line. */ > + preprocess_compact_syntax (loc, alt_no, line, last_line); > + new_templ.append (line); > + last_line =3D line; > + > + /* Normal "*..." syntax expects the closing quote to be on the fin= al > + line of asm, whereas we allow the closing "}" to be on its own l= ine. > + Postpone copying the '\n' until we know that there is another > + alternative in the list. */ > + while (templ[0] =3D=3D '\n' || ISSPACE (*templ)) > + templ++; The separate check for '\n' isn't necessary. Sorry for all the micro-comments. Thanks, Richard > + ++alt_no; > + } > + > + /* Write the constraints and attributes into their proper places. */ > + if (convec.size () > 0) > + add_constraints (x, loc, convec); > + > + if (attrvec.size () > 0) > + { > + index =3D add_attributes (x, loc, attrvec); > + if (index < attrvec.size ()) > + fatal_at (loc, "could not find set_attr for attribute %s", > + attrvec[index].name.c_str ()); > + } > + > + /* Copy over the new un-constrainified template. */ > + XTMPL (x, templ_index) =3D xstrdup (new_templ.c_str ()); > + > + /* Register for later checks during iterator expansions. */ > + compact_syntax.add (x); > +} > + > /* Process a top level rtx in some way, queuing as appropriate. */ > > static void > @@ -553,10 +1077,12 @@ process_rtx (rtx desc, file_location loc) > switch (GET_CODE (desc)) > { > case DEFINE_INSN: > + convert_syntax (desc, loc); > queue_pattern (desc, &define_insn_tail, loc); > break; > > case DEFINE_COND_EXEC: > + convert_syntax (desc, loc); > queue_pattern (desc, &define_cond_exec_tail, loc); > break; > > @@ -631,6 +1157,7 @@ process_rtx (rtx desc, file_location loc) > attr =3D XVEC (desc, split_code + 1); > PUT_CODE (desc, DEFINE_INSN); > XVEC (desc, 4) =3D attr; > + convert_syntax (desc, loc); > > /* Queue them. */ > insn_elem =3D queue_pattern (desc, &define_insn_tail, loc);