jyknight added inline comments.
================ Comment at: clang/lib/AST/Stmt.cpp:646-648 + // Labels are placed after "InOut" operands. Adjust accordingly. + if (IsLabel) + N += getNumPlusOperands(); ---------------- 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69876/new/ https://reviews.llvm.org/D69876 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
