> -----Original Message-----
> From: Richard Sandiford <[email protected]>
> Sent: 29 September 2026 15:01
> To: Tamar Christina <[email protected]>
> Cc: [email protected]; nd <[email protected]>; [email protected];
> Richard Earnshaw <[email protected]>;
> [email protected]
> Subject: Re: [patch][gcc 1/5]middle-end: support multi-character output
> operand modifiers
>
> Tamar Christina <[email protected]> writes:
> > Today output operand modifiers are limited to a single character, as in
> > %w0 or %c1. That works, but it makes the modifier namespace very small
> > and encourages target ports either to use cryptic letters or to fall back
> > to C output blocks for cases that are really just target-specific
> > printing.
> >
> > This patch adds a longer spelling for the same mechanism:
> >
> > %[name]0
> >
> > The bracketed spelling is intentionally familiar rather than completely new:
> > extended asm already uses %[Value] to refer to an operand with a symbolic
> name,
> > as in:
> >
> > asm ("add %[out], %[in], %[imm]"
> > : [out] "=r" (out)
> > : [in] "r" (in), [imm] "I" (1));
> >
> > That existing form keeps its meaning. A bare %[Value] is still resolved by
> > the front end as a symbolic operand reference. The new print-operand form
> is
> > recognized when the closing bracket is followed by an operand number, as in
> > %[name]0.
> >
> > The modifier syntax can also be combined with symbolic operand names. In
> user
> > asm, %[modifier:Value] means "print operand Value using the multi-
> character
> > modifier named modifier". The front end resolves Value to an operand
> number,
> > so final output sees the same thing it would have seen for %[modifier:N].
> For
> > example:
> >
> > asm ("mov %[w:out], %[w:in]"
> > : [out] "=r" (out)
> > : [in] "r" (in));
> >
> > is resolved as if the template had used %[w]0 and %[w]1.
> >
> > GCC parses the bracketed modifier name, interns it, and passes
> PRINT_OPERAND an
> > opaque integer code for it. Old-style modifiers keep their existing
> > values, so
> > existing target hooks continue to see 'w', 'c', etc. for the
> > single-character
> > forms. Backends that want to handle the new form can call
> > output_operand_modifier_name (code) to recover "name".
> >
> > For example, a machine description can now write something like:
> >
> > "extract\t%[lowpart]0, %1"
> >
> > or if preferred
> >
> > "extract\t%[lowpart:0], %1"
> >
> > and the target's PRINT_OPERAND hook can handle "lowpart" as a named
> modifier.
> > The parser also allows comma-separated nonempty components in the
> brackets, but
> > deliberately leaves the interpretation of those components to the target.
> >
> > The patch also records whether an extended modifier came from user asm
> or from
> > an internal machine-description output template. Targets can use
> > output_operand_modifier_user_p (code) to reject internal-only names in
> > user-facing asm while still using the clearer names internally.
> >
> > This addresses Richard Earnshaw's suggestion by keeping the target hook
> > interface as an int code and mapping the longer spelling onto values outside
> > the single-character range. Targets that do not opt in do not need a new
> hook
> > or a new parser path, and existing hard-coded character uses continue to
> work.
> >
> > It also addresses Richard Sandiford's feedback by giving the new %[...]
> > syntax
> > its own namespace rather than automatically exposing every single-letter
> > modifier through the new spelling. The user-asm bit lets a target decide
> which
> > named modifiers are public and which are only for compiler-generated
> templates.
> > The comma-separated spelling is accepted as a target-defined modifier
> string,
> > so a port can later treat, say, %[gpr,zero]0 as a composition of
> > target-specific printing choices without requiring more middle-end syntax.
> >
> > Bootstrapped Regtested on aarch64-none-linux-gnu,
> > arm-none-linux-gnueabihf, x86_64-pc-linux-gnu
> > -m32, -m64 and no issues.
> >
> > OK for master?
> >
> > Thanks,
> > Tamar
>
> Thanks for doing this. It mostly looks good to me FWIW, but some
> comments below:
>
Thanks for the review!
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > index
> 88213d25a899ed5bff25af732220c54d2359b492..1e6f5ddb0a815101f6b6
> c57bc8f610766e035895 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -12057,7 +12057,14 @@ instead of simply @samp{%2}).
> @ref{GenericOperandmodifiers,
> > Generic Operand modifiers} lists the modifiers that are available
> > on all targets. Other modifiers are hardware dependent.
> > For example, the list of supported modifiers for x86 is found at
> > -@ref{x86Operandmodifiers,x86 Operand modifiers}.
> > +@ref{x86Operandmodifiers,x86 Operand modifiers}. Targets can also
> provide
> > +multi-character modifiers, written as @samp{%[@var{modifier}:2]}.
> > +
>
> Very minor, but: I think it'd read better if this was part of the following
> paragraph instead, so that it's more obvious that new paragraph is referring
> specifically to the new syntax.
>
> > +If the operand has a symbolic name, write
> > +@samp{%[@var{modifier}:@var{asmSymbolicName}]}. The older spelling
> > +@samp{%[@var{modifier}]2} is also accepted for numbered operands.
> > +@samp{%[@var{name}]} without a modifier separator continues to mean
> the
> > +operand named @var{name}.
> >
> > If the C code that follows the @code{asm} makes no use of any of the
> output
> > operands, use @code{volatile} for the @code{asm} statement to prevent
> the
> > [...]
> > diff --git a/gcc/final.cc b/gcc/final.cc
> > index
> b6be9be99b7f12d69345de3c4edf99d33dfa6087..9953e1d44d9d66672b5d
> aacc5a6200de3ba1df8c 100644
> > --- a/gcc/final.cc
> > +++ b/gcc/final.cc
> > @@ -84,6 +84,7 @@ along with GCC; see the file COPYING3. If not see
> > #include "common/common-target.h"
> > #include "diagnostic.h"
> > #include "diagnostics/file-cache.h"
> > +#include "selftest.h"
> >
> > #include "dwarf2out.h"
> >
> > @@ -3395,6 +3396,216 @@ do_assembler_dialects (const char *p, int
> *dialect)
> > }
> > #endif
> >
> > +/* Return true if the half-open range [START, END) contains only decimal
> > + digits, parsing it as an operand number into OPNUM. */
> > +
> > +static bool
> > +parse_operand_number (const char *start, const char *end,
> > + unsigned long &opnum)
>
> It'd be good to export this and reuse it instead of asm_operand_number_p.
> Maybe that would mean moving it too, although I'm not sure where.
>
> > +{
> > + if (start == end)
> > + return false;
> > +
> > + unsigned long result = 0;
> > + for (const char *q = start; q != end; ++q)
> > + {
> > + if (!ISDIGIT (*q))
> > + return false;
> > + result = result * 10 + (*q - '0');
> > + }
> > + opnum = result;
> > + return true;
> > +}
> > [...]
> > +#if CHECKING_P
> > +
> > +namespace selftest {
> > +
> > +static void
> > +test_parse_bracketed_operand ()
> > +{
> > + bracketed_operand operand;
> > + const char *p = "[10]";
> > + ASSERT_TRUE (parse_bracketed_operand (&p, operand));
> > + ASSERT_FALSE (operand.has_modifier);
> > + ASSERT_EQ (10UL, operand.opnum);
> > + ASSERT_EQ ('\0', *p);
> > +
> > + p = "[fmt:11]";
> > + ASSERT_TRUE (parse_bracketed_operand (&p, operand));
> > + ASSERT_TRUE (operand.has_modifier);
> > + ASSERT_TRUE (operand.modifier == "fmt");
> > + ASSERT_EQ (11UL, operand.opnum);
> > + ASSERT_EQ ('\0', *p);
> > +
> > + p = "[fmt]12";
> > + ASSERT_TRUE (parse_bracketed_operand (&p, operand));
> > + ASSERT_TRUE (operand.has_modifier);
> > + ASSERT_TRUE (operand.modifier == "fmt");
> > + ASSERT_EQ (12UL, operand.opnum);
> > + ASSERT_EQ ('\0', *p);
> > +
> > + p = "[gpr,zero:13]";
> > + ASSERT_TRUE (parse_bracketed_operand (&p, operand));
> > + ASSERT_TRUE (operand.has_modifier);
> > + ASSERT_TRUE (operand.modifier == "gpr,zero");
> > + ASSERT_EQ (13UL, operand.opnum);
> > + ASSERT_EQ ('\0', *p);
> > +
> > + p = "[fmt]";
> > + ASSERT_FALSE (parse_bracketed_operand (&p, operand));
> > + p = "[fmt:]";
> > + ASSERT_FALSE (parse_bracketed_operand (&p, operand));
> > + p = "[]0";
> > + ASSERT_FALSE (parse_bracketed_operand (&p, operand));
> > +}
> > +
> > +void
> > +final_cc_tests ()
> > +{
> > + test_parse_bracketed_operand ();
> > +}
> > +
> > +} // namespace selftest
> > +
> > +#endif /* CHECKING_P */
>
> I think self-tests are conventially at the end of the file. Putting it
> in the middle would artificially restrict where the code under test can go.
>
> > @@ -3488,11 +3699,37 @@ output_asm_insn (const char *templ, rtx
> *operands)
> > p++;
> > fprintf (asm_out_file, "%d", insn_counter);
> > }
> > + else if (*p == '[')
> > + {
> > + bracketed_operand operand;
> > + if (!parse_bracketed_operand (&p, operand))
> > + output_operand_lossage ("invalid %%[...] operand modifier");
> > +
> > + unsigned long opnum = operand.opnum;
> > + if (this_is_asm_operands && opnum >= insn_noperands)
> > + {
> > + /* Force the opnum in bounds to a bogus location. */
> > + opnum = MAX_RECOG_OPERANDS;
> > + output_operand_lossage ("operand number out of range");
> > + }
> > + else if (operand.has_modifier)
> > + output_operand (operands[opnum],
> > + output_operand_modifier_code
> > + (operand.modifier, this_is_asm_operands));
> > + else
> > + output_operand (operands[opnum], 0);
> > +
> > + if (!opoutput[opnum])
> > + oporder[ops++] = opnum;
> > + opoutput[opnum] = 1;
> > +
> > + c = *p;
> > + }
> > /* % followed by a letter and some digits
> > outputs an operand in a special way depending on the letter.
> > Letters `acln' are implemented directly.
> > Other letters are passed to `output_operand' so that
> > - the TARGET_PRINT_OPERAND hook can define them. */
> > + the selected target operand hook can define them. */
>
> Is this comment change left over from an earlier version? I wasn't sure
> what the "selected target operand hook" was referring to.
>
> > else if (ISALPHA (*p))
> > {
> > int letter = *p++;
> > @@ -3522,7 +3759,7 @@ output_asm_insn (const char *templ, rtx
> *operands)
> > if (letter2 == 'c' || CONSTANT_ADDRESS_P
> (operands[opnum]))
> > output_addr_const (asm_out_file, operands[opnum]);
> > else
> > - output_operand (operands[opnum], 'c');
> > + output_operand (operands[opnum], letter);
> > }
> > else if (letter == 'n')
> > {
> > @@ -3570,9 +3807,12 @@ output_asm_insn (const char *templ, rtx
> *operands)
> > }
> > /* % followed by punctuation: output something for that
> > punctuation character alone, with no operand. The
> > - TARGET_PRINT_OPERAND hook decides what is actually done. */
> > + selected target operand hook decides what is actually done. */
> > else if (targetm.asm_out.print_operand_punct_valid_p ((unsigned
> char) *p))
> > - output_operand (NULL_RTX, *p++);
> > + {
> > + const char *ptr = p++;
> > + output_operand (NULL_RTX, *ptr);
> > + }
> > else
> > output_operand_lossage ("invalid %%-code");
> > break;
>
> Same question for the changes in the two hunks above. I wasn't sure why
> they were necessary.
Yeah, both the change above and this are left overs from when the code passed
char**.
I had clearly missed them during cleanup :/
>
> > [...]
> > @@ -870,6 +875,67 @@ check_unique_operand_names (tree outputs, tree
> inputs, tree labels)
> > return false;
> > }
> >
> > +/* P points to a '%' in an asm template. Return the '[' for the symbolic
> > + operand name that should be resolved, or null if this % code does not
> > + contain a symbolic operand name.
> > +
> > + The %[...] syntax has two uses in user asm: %[name] refers to a named
> > + operand, whereas %[modifier:N] and %[modifier]N are multi-character
> output
> > + modifiers. When both a modifier and a symbolic operand name are
> needed,
> > + use %[modifier:name]. */
> > +
> > +static bool
> > +asm_operand_number_p (const char *start, const char *end)
> > +{
> > + if (start == end)
> > + return false;
> > +
> > + for (const char *q = start; q != end; ++q)
> > + if (!ISDIGIT (*q))
> > + return false;
> > + return true;
> > +}
> > +
>
> Missing function comment.
>
> > +static const char *
> > +asm_operand_name_start (const char *p)
> > +{
> > + gcc_checking_assert (p[0] == '%');
> > +
> > + if (p[1] == '[')
> > + {
> > + const char *q = strchr (p + 2, ']');
> > + if (q)
> > + {
> > + const char *colon = nullptr;
> > + for (const char *r = p + 2; r != q; ++r)
> > + if (*r == ':')
> > + {
> > + colon = r;
> > + break;
> > + }
> > +
> > + if (colon)
> > + {
> > + if (asm_operand_number_p (colon + 1, q))
> > + return NULL;
> > + return colon + 1;
> > + }
> > +
> > + if (asm_operand_number_p (p + 2, q))
> > + return NULL;
> > + if (ISDIGIT (q[1]))
> > + return NULL;
>
> I was going to suggest that we disallow the trailing digit syntax in
> asms (and perhaps then in general), since it would reinterpret what
> "%[name]0" means. However, it seems that what "%[name]0" currently
> means isn't useful: "%[name]" is replaced with "%<opnum>" and so we get
> a double-digit operand number. For example:
>
> void f() {
> asm volatile ("foo %[number]0" :: [x] "i" (1), [number] "r" (100));
> }
>
> is treated as equivalent to "foo %10" and so gives an out-of-range error.
>
> Still, I'm somewhat sceptical of the "%[modifier]opno" syntax in general.
> It's a new syntax with no existing users and I don't see why it would
> ever be preferred over the more explicit %[modifier:opno]".
>
Yeah, Agreed, Richard Earnshaw brought up the same thing internally after
I posted the patch and I arrived at the same conclusion as you. I was on the
edge about removing it but happy to do so to avoid ambiguity. My original
reason for keeping it was because it made the line less "funky" when e.g. the
syntax is used for lanes. Like [%[foo],0] looked easier to read than [%[foo:0]]
but I realize this is completely subjective, and for the aarch64 patches I went
with %[modifier:operand] anyway.
That syntax also lends more naturally to the other extension I want to do where
The modifier can consume multiple operands, so %[foo:0,1].
So I'll drop it in the next respin.
> > + }
> > + return p + 1;
> > + }
> > +
> > + if (ISALPHA (p[1])
> > + && (p[2] == '[' || (ISALPHA (p[2]) && p[3] == '[')))
> > + return p + 2 + (p[2] != '[');
>
> Heh. I hadn't realised that we were so loose about where the modifier ended.
> But clearly that's pre-existing and the patch is doing the right thing by
> preserving it.
>
> > +
> > + return NULL;
> > +}
> > +
> > /* Resolve the names of the operands in *POUTPUTS and *PINPUTS to
> numbers,
> > and replace the name expansions in STRING and in the constraints to
> > those numbers. This is generally done in the front end while creating
> > @@ -905,10 +971,7 @@ resolve_asm_operand_names (tree string, tree
> outputs, tree inputs, tree labels)
> > c = TREE_STRING_POINTER (string);
> > while ((c = strchr (c, '%')) != NULL)
> > {
> > - if (c[1] == '[')
> > - break;
> > - else if (ISALPHA (c[1])
> > - && (c[2] == '[' || (ISALPHA (c[2]) && c[3] == '[')))
> > + if (asm_operand_name_start (c))
> > break;
> > else
> > {
> > @@ -920,26 +983,23 @@ resolve_asm_operand_names (tree string, tree
> outputs, tree inputs, tree labels)
> > if (c)
> > {
> > /* OK, we need to make a copy so we can perform the substitutions.
> > - Assume that we will not need extra space--we get to remove '['
> > - and ']', which means we cannot have a problem until we have more
> > - than 999 operands. */
> > - buffer = xstrdup (TREE_STRING_POINTER (string));
> > + Allocate some spare room for %[modifier:name], where the
> replacement
> > + number can be longer than NAME. */
> > + size_t length = strlen (TREE_STRING_POINTER (string));
> > + buffer = XNEWVEC (char, length * 2 + 1);
>
> Not sure about the * 2. More below.
>
> > + memcpy (buffer, TREE_STRING_POINTER (string), length + 1);
> > p = buffer + (c - TREE_STRING_POINTER (string));
> >
> > while ((p = strchr (p, '%')) != NULL)
> > {
> > - if (p[1] == '[')
> > - p += 1;
> > - else if (ISALPHA (p[1]) && p[2] == '[')
> > - p += 2;
> > - else if (ISALPHA (p[1]) && ISALPHA (p[2]) && p[3] == '[')
> > - p += 3;
> > - else
> > + const char *name_start = asm_operand_name_start (p);
> > + if (!name_start)
> > {
> > p += 1 + (p[1] == '%');
> > continue;
> > }
> >
> > + p += name_start - p;
>
> Isn't that just "p = name_start;"? If so, we could use name_start directly
> in the statement below instead.
>
*facepalm*
> > p = resolve_operand_name_1 (p, outputs, inputs, labels);
>
> Since this patch is rewriting this code anyway, it would be good to make
> it work regardless of MAX_RECOG_OPERANDS (which won't exist once the
> wasm backend goes in).
>
> So I think at this point we should just abandon the preallocation approach
> and use an obstack instead. That is: iterate over the original input
> string, growing the obstack with output characters as we go. That
> would avoid the need to calculate a length in advance, would cope
> with any number of digits, and would remove the complex shuffle in
> resolve_operand_name_1.
>
Some of the feedback make me wonder since I'm already changing the parsing
anyway whether I should merge my follow up into this one as well.
What I wanted was to flip the operand and modifier parse order for the new
syntax, such that the operand RTL are available in the print_operands function.
This allows us to simplify printing by not needing iterators to convey the
content
of e.g. constant RTL or the types. For instance printing a scalar register no
longer
requires us to explicitly use a different modifier for a particular register,
we could
infer it from the mode.
Should I try to get that In now or delay as a follow up?
Thanks,
Tamar
> Thanks,
> Richard
>
> > }
> >
> > @@ -958,24 +1018,40 @@ resolve_asm_operand_names (tree string, tree
> outputs, tree inputs, tree labels)
> > static char *
> > resolve_operand_name_1 (char *p, tree outputs, tree inputs, tree labels)
> > {
> > - char *q;
> > + char *q, *name_start, *replace_start, *replace_end;
> > int op, op_inout;
> > tree t;
> >
> > - /* Collect the operand name. */
> > - q = strchr (++p, ']');
> > + /* Collect the operand name. P either points to the '[' in %[name],
> > + or to the first character of NAME in %[modifier:name]. */
> > + if (*p == '[')
> > + {
> > + replace_start = p;
> > + name_start = p + 1;
> > + q = strchr (name_start, ']');
> > + replace_end = q ? q + 1 : nullptr;
> > + }
> > + else
> > + {
> > + replace_start = p;
> > + name_start = p;
> > + q = strchr (name_start, ']');
> > + replace_end = q;
> > + }
> > +
> > if (!q)
> > {
> > error ("missing close brace for named operand");
> > - return strchr (p, '\0');
> > + return strchr (name_start, '\0');
> > }
> > + char end_marker = *q;
> > *q = '\0';
> >
> > /* Resolve the name to a number. */
> > for (op_inout = op = 0, t = outputs; t ; t = TREE_CHAIN (t), op++)
> > {
> > tree name = TREE_PURPOSE (TREE_PURPOSE (t));
> > - if (name && strcmp (TREE_STRING_POINTER (name), p) == 0)
> > + if (name && strcmp (TREE_STRING_POINTER (name), name_start) == 0)
> > goto found;
> > tree constraint = TREE_VALUE (TREE_PURPOSE (t));
> > if (constraint && strchr (TREE_STRING_POINTER (constraint), '+') !=
> NULL)
> > @@ -984,35 +1060,89 @@ resolve_operand_name_1 (char *p, tree
> outputs, tree inputs, tree labels)
> > for (t = inputs; t ; t = TREE_CHAIN (t), op++)
> > {
> > tree name = TREE_PURPOSE (TREE_PURPOSE (t));
> > - if (name && strcmp (TREE_STRING_POINTER (name), p) == 0)
> > + if (name && strcmp (TREE_STRING_POINTER (name), name_start) == 0)
> > goto found;
> > }
> > op += op_inout;
> > for (t = labels; t ; t = TREE_CHAIN (t), op++)
> > {
> > tree name = TREE_PURPOSE (t);
> > - if (name && strcmp (TREE_STRING_POINTER (name), p) == 0)
> > + if (name && strcmp (TREE_STRING_POINTER (name), name_start) == 0)
> > goto found;
> > }
> >
> > - error ("undefined named operand %qs", identifier_to_locale (p));
> > + error ("undefined named operand %qs", identifier_to_locale
> (name_start));
> > op = 0;
> >
> > found:
> > - /* Replace the name with the number. Unfortunately, not all libraries
> > - get the return value of sprintf correct, so search for the end of the
> > - generated string by hand. */
> > - sprintf (--p, "%d", op);
> > - p = strchr (p, '\0');
> > + *q = end_marker;
> > + char number[HOST_BITS_PER_INT * 3 / 10 + 3];
> > + sprintf (number, "%d", op);
> > + size_t number_len = strlen (number);
> > + memmove (replace_start + number_len, replace_end,
> > + strlen (replace_end) + 1);
> > + memcpy (replace_start, number, number_len);
> > +
> > + return replace_start + number_len;
> > +}
> > +
> > +#if CHECKING_P
> > +
> > +namespace selftest {
> > +
> > +static tree
> > +build_asm_operand (const char *name, const char *constraint)
> > +{
> > + tree name_tree = name ? build_string (strlen (name), name) : NULL_TREE;
> > + tree constraint_tree = build_string (strlen (constraint), constraint);
> > + return tree_cons (tree_cons (name_tree, constraint_tree, NULL_TREE),
> > + integer_zero_node, NULL_TREE);
> > +}
> >
> > - /* Verify the no extra buffer space assumption. */
> > - gcc_assert (p <= q);
> > +static tree
> > +build_asm_label (const char *name)
> > +{
> > + return tree_cons (build_string (strlen (name), name), integer_zero_node,
> > + NULL_TREE);
> > +}
> >
> > - /* Shift the rest of the buffer down to fill the gap. */
> > - memmove (p, q + 1, strlen (q + 1) + 1);
> > +static void
> > +test_resolve_asm_operand_names ()
> > +{
> > + tree outputs = build_asm_operand ("out", "=r");
> > + tree inputs = build_asm_operand ("in", "r");
> > + tree labels = build_asm_label ("lab");
> > +
> > + const char *templ = "%[out] %w[out] %cc[in] %l[lab]";
> > + tree string = build_string (strlen (templ), templ);
> > + tree result = resolve_asm_operand_names (string, outputs, inputs,
> > labels);
> > + ASSERT_STREQ ("%0 %w0 %cc1 %l2", TREE_STRING_POINTER (result));
> > +
> > + templ = "%[fmt]0 %[fmt]1";
> > + string = build_string (strlen (templ), templ);
> > + result = resolve_asm_operand_names (string, outputs, inputs, labels);
> > + ASSERT_STREQ ("%[fmt]0 %[fmt]1", TREE_STRING_POINTER (result));
> > +
> > + templ = "%[10] %[fmt:0] %[fmt:1]";
> > + string = build_string (strlen (templ), templ);
> > + result = resolve_asm_operand_names (string, outputs, inputs, labels);
> > + ASSERT_STREQ ("%[10] %[fmt:0] %[fmt:1]", TREE_STRING_POINTER
> (result));
> > +
> > + templ = "%[fmt:out] %[fmt:in]";
> > + string = build_string (strlen (templ), templ);
> > + result = resolve_asm_operand_names (string, outputs, inputs, labels);
> > + ASSERT_STREQ ("%[fmt:0] %[fmt:1]", TREE_STRING_POINTER (result));
> > +}
> >
> > - return p;
> > +void
> > +stmt_cc_tests ()
> > +{
> > + test_resolve_asm_operand_names ();
> > }
> > +
> > +} // namespace selftest
> > +
> > +#endif /* CHECKING_P */
> >
>
>
> >
> > /* Generate RTL to return directly from the current function.
