[PATCH] D69876: Allow output constraints on "asm goto"

[Bill Wendling via Phabricator via cfe-commits]

void marked an inline comment as done.
void added a comment.

In D69876#1812724 <https://reviews.llvm.org/D69876#1812724>, @jyknight wrote:

> Reopening, since this didn't actually land yet. BTW, this review still has the wrong contents in the latest uploaded-diff (showing llvm changes, not clang changes).


*Pinches bridge of nose*
*Curses under breath*



================
Comment at: clang/lib/AST/Stmt.cpp:646-648
+      // Labels are placed after "InOut" operands. Adjust accordingly.
+      if (IsLabel)
+        N += getNumPlusOperands();
----------------
jyknight wrote:
> void wrote:
> > jyknight wrote:
> > > I'm confused about this part. Why isn't the "N" specified in the assembly string already the correct value for the labels? Is the ordering we use internally and that users use externally not the same? I'm assuming your code here is correct, just I'm not understanding, so probably an improved comment would be nice.
> > The LLVM-specific ordering that I saw was something like:
> > 
> >   `<output constraints>,<input constraints>,<plus constraints>,<optional X>,<other constraints>`
> > 
> > The `<plus constraints>` is empty when there are no output constraints. This just makes up for this. It's probably better to reverse the `<optional X>` and `<plus constraints>` parts, but I didn't know how pervasive the order's assumption is in the code.
> Oh, right -- in llvm IR, the "plus" constraints don't exist -- we specify those as separate-but-linked output and input constraints.
> 
> But thinking about this more...I think this code is actually incorrect. It should be translating the numbers based on where it finds the matching argument-number, not on the 'l' character being present. (And it should do so for named-arguments too).
> 
> So, yes, I think it'd be better to just swap the order, and move the plus-operand tied-inputs to the end of the arglist, where they don't affect numbering. I don't _think_ reversing the order will cause other issues.
I agree that moving them would make the most sense. Let me do some experiments to see if it breaks anything.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69876/new/

https://reviews.llvm.org/D69876