> -----Original Message----- > From: Richard Sandiford <[email protected]> > Sent: 30 September 2026 22:14 > 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: > >> [...] > >> 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]. > > Ah, interesting. > > >> > [...] > >> > @@ -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; > >> > [...] > >> > 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. > > Perhaps I'm misunderstanding, but I think this is already possible. > I'd always assumed that it was a policy decision not to do it for AArch64, > rather than a restriction of the infrastructure. The port could have had > a modifier that infers the choice between %b, %h, %s, %d, or %q from the > mode of the register and avoided using %<Vetype>. (%b etc. still seem > valuable, since there are outlying cases where being able to force a > particular register prefix is useful.) > > That was before my time though. > > For SVE, we mostly embedded the size suffixes in the asm string for > consistency with Adv SIMD, but we did deviate from it for printing > register ranges: > > case 0: > if (x == NULL) > { > output_operand_lossage ("missing operand"); > return; > } > > switch (GET_CODE (x)) > { > case CONST_STRING: > { > asm_fprintf (f, "%s", XSTR (x, 0)); > break; > } > case REG: > if (aarch64_sve_data_mode_p (GET_MODE (x))) > { > if (REG_NREGS (x) == 1) > asm_fprintf (f, "z%d", REGNO (x) - V0_REGNUM); > else > { > char suffix > = sizetochar (GET_MODE_UNIT_BITSIZE (GET_MODE (x))); > asm_fprintf (f, "{z%d.%c - z%d.%c}", > REGNO (x) - V0_REGNUM, suffix, > END_REGNO (x) - V0_REGNUM - 1, suffix); > } > } > else > asm_fprintf (f, "%s", reg_names [REGNO (x)]); > break; > > Maybe that was the wrong decision and we should have added a single > "Z register with suffix" modifier for both single and multiple registers. > (Of course, we do need to print unsuffixed Z registers for MOVPRFX, > and also need to print P registers without an element size suffix in > several contexts.)
Ah, true. When I was looking at it my issue was trying to do the lane endian swapping as a print_operand. And the issue there of course is that the value isn't a reg but a CONST_INT where sadly we have no mode. And you have the many %s,d,h etc print modifier for printing the different scalar types. I'm hoping to replace these with %[scalar] in cases where we don't play tricks with the register file. I haven't tried that one to see if it works out better yet.. V3 finished in CI, will send it up today after I update the changelog. Thanks for the reviews! Tamar > > Thanks, > Richard
