On 06/06/2023 13:49, Richard Sandiford via Gcc-patches wrote:
Tamar Christina <tamar.christ...@arm.com> writes:
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 +702,57 @@ process_template (class data *d, const char
*template_code)
if (sp != ep)
message_at (d->loc, "trailing whitespace in output template");
- while (cp < sp)
+ /* Check for any unexpanded iterators. */
+ if (bp[0] != '*' && d->compact_syntax_p)
I assume the bp[0] != '*' condition skips the check for C code blocks.
Genuine question, but are you sure we want that? C code often includes asm
strings (in quotes), such as for the SVE CNT[BHWD] example.
Extending the check would mean that any use of <...> for C++ templates will
need to be quoted, but explicit instantiation is pretty rare in .md files. It
would
also look weird for conditions.
Either way is fine, just asking.
I excluded it entirely to avoid also running afoul of the binary operators. So
e.g.
* a < b && b > c ? foo : bar shouldn't trigger it. It seemed more trouble
than it's
worth to try to get correct.
Yeah. I agree it's probably better to skip.
+ }
+
+ /* Adds a character to the end of the string. */ void add (char
+ c) {
+ con += c;
+ }
+
+ /* Output the string in the form of a brand-new char *, then effectively
+ clear the internal string by resetting len to 0. */ char * out
+ ()
Formatting: no need for a space before "out".
+ {
+ /* Final character is always a trailing comma, so strip it out.
+ */
trailing ',', ';' or ']', rather than just a comma?
Ah no, this is a bit of a lazy intercalate, when the alternatives are pushed in
it's
not easy to tell how many there will be (because we don't keep track of it in
this part),
so we just always add a trailing "," and ignore the last char on output.
Validation of the
alternative counts themselves is done later by the normal machinery.
Ah, I get it now, thanks.
+ }
+
+ 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 = GET_CODE (x) == DEFINE_INSN ? 4 : 3;
+ vec_conlist missing;
+
+ /* This is an O(n*m) loop but it's fine, both n and m will always be very
+ small. */
Agreed that quadraticness isn't a problem. But I wonder how many people
would write an explicit placeholder set_attr. Unlike match_operand and
match_scratch, a placeholder set_attr doesn't carry any additional
information.
It might be simpler to drop add_attributes and add all attributes
unconditionally in this function instead. If the user tries to specify the same
attribute using both syntaxes, the pattern would end up with two definitions
of the same attribute, which ought to be flagged by existing code.
This was done to support the (in arm backend) common thing of having attributes
which are either too complex to add inline in the new syntax or that just
repeat a
value.
i.e. it's to allow cases like this:
[(set_attr "length")
(set_attr "predicable" "yes")
(set_attr "predicable_short_it")
(set_attr "arch")
(set (attr "type") (if_then_else (match_operand 2 "const_int_operand" "")
(const_string "alu_imm")
(const_string "alu_sreg")))
Where your attrs contains:
{@ [cons: =0, 1, 2; attrs: length, predicable_short_it, arch]
Yeah, agree it needs to be possible to define things like "type"
in this way.
You also want it for the case where every alternative takes the same
value, eg the "predicable - yes" attr.
R.
However you're right, I could simply say that you must omit the set_attr in
attrs and just
merge the two lists? I think that's what you were alluding to?
Yeah, that's right. Or just concatenate them and rely on later
error checking (which should give reasonable diagnostics).
Thanks,
Richard
