[clang] number labels in asm goto strings after tied inputs

I noticed that the following case would compile in Clang but not GCC:
    void *x(void) {
      void *p = &&foo;
      asm goto ("# %0\n\t# %l1":"+r"(p):::foo);
      foo:;
      return p;
    }

Changing the output template above from %l2 would compile in GCC but not
Clang.

This demonstrates that when using tied outputs (say via the "+r" output
constraint), the hidden inputs occur or are numbered BEFORE the labels,
at least with GCC.

In fact, GCC does denote this in its documentation:
https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Extended-Asm.html#Goto-Labels

> Output operand with constraint modifier ‘+’ is counted as two operands
> because it is considered as one output and one input operand.

For the sake of compatibility, I think it's worthwhile to just make this
change.

It's better to use symbolic names for compatibility (especially now
between released version of Clang that support asm goto with outputs).
ie. %l1 from the above would be %l[foo]. The GCC docs also make this
recommendation.

Also, I cleaned up some cruft in GCCAsmStmt::getNamedOperand. AFAICT,
NumPlusOperands was no longer used, though I couldn't find which commit
didn't clean that up correctly.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98096
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103640
Link: https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Extended-Asm.html#Goto-Labels

Reviewed By: void

Differential Revision: https://reviews.llvm.org/D115471

clang/docs/LanguageExtensions.rst

@@ -1471,6 +1471,33 @@ Using outputs on an indirect branch may result in undefined behavior. For
 example, in the function above, use of the value assigned to `y` in the `err`
 block is undefined behavior.
 
+When using tied-outputs (i.e. outputs that are inputs and outputs, not just
+outputs) with the `+r` constraint, there is a hidden input that's created
+before the label, so numeric references to operands must account for that.
+
+.. code-block:: c++
+
+  int foo(int x) {
+      // %0 and %1 both refer to x
+      // %l2 refers to err
+      asm goto("# %0 %1 %l2" : "+r"(x) : : : err);
+      return x;
+    err:
+      return -1;
+  }
+
+This was changed to match GCC in clang-13; for better portability, symbolic
+references can be used instead of numeric references.
+
+.. code-block:: c++
+
+  int foo(int x) {
+      asm goto("# %[x] %l[err]" : [x]"+r"(x) : : : err);
+      return x;
+    err:
+      return -1;
+  }
+
 Query for this feature with ``__has_extension(gnu_asm_goto_with_outputs)``.
 
 Objective-C Features

clang/docs/ReleaseNotes.rst

@@ -148,7 +148,6 @@ C Language Changes in Clang
 - Support for ``__attribute__((error("")))`` and
   ``__attribute__((warning("")))`` function attributes have been added.
 - The maximum allowed alignment has been increased from 2^29 to 2^32.
-
 - Clang now supports the ``_BitInt(N)`` family of bit-precise integer types
   from C23. This type was previously exposed as ``_ExtInt(N)``, which is now a
   deprecated alias for ``_BitInt(N)`` (so diagnostics will mention ``_BitInt``
@@ -158,6 +157,13 @@ C Language Changes in Clang
   modes. Note: the ABI for ``_BitInt(N)`` is still in the process of being
   stabilized, so this type should not yet be used in interfaces that require
   ABI stability.
+- When using ``asm goto`` with outputs whose constraint modifier is ``"+"``, we
+  now change the numbering of the labels to occur after hidden tied inputs for
+  better compatibility with GCC.  For better portability between different
+  compilers and versions, symbolic references rather than numbered references
+  should be preferred. See
+  `this thread <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103640>` for more
+  info.
 
 C++ Language Changes in Clang
 -----------------------------

clang/lib/AST/Stmt.cpp

@@ -568,21 +568,20 @@ void GCCAsmStmt::setOutputsAndInputsAndClobbers(const ASTContext &C,
 /// translate this into a numeric value needed to reference the same operand.
 /// This returns -1 if the operand name is invalid.
 int GCCAsmStmt::getNamedOperand(StringRef SymbolicName) const {
-  unsigned NumPlusOperands = 0;
-
   // Check if this is an output operand.
-  for (unsigned i = 0, e = getNumOutputs(); i != e; ++i) {
+  unsigned NumOutputs = getNumOutputs();
+  for (unsigned i = 0; i != NumOutputs; ++i)
     if (getOutputName(i) == SymbolicName)
       return i;
-  }
 
-  for (unsigned i = 0, e = getNumInputs(); i != e; ++i)
+  unsigned NumInputs = getNumInputs();
+  for (unsigned i = 0; i != NumInputs; ++i)
     if (getInputName(i) == SymbolicName)
-      return getNumOutputs() + NumPlusOperands + i;
+      return NumOutputs + i;
 
   for (unsigned i = 0, e = getNumLabels(); i != e; ++i)
     if (getLabelName(i) == SymbolicName)
-      return i + getNumOutputs() + getNumInputs();
+      return NumOutputs + NumInputs + getNumPlusOperands() + i;
 
   // Not found.
   return -1;

clang/lib/CodeGen/CGStmt.cpp

@@ -2542,6 +2542,14 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
     Constraints += InputConstraint;
   }
 
+  // Append the "input" part of inout constraints.
+  for (unsigned i = 0, e = InOutArgs.size(); i != e; i++) {
+    ArgTypes.push_back(InOutArgTypes[i]);
+    ArgElemTypes.push_back(InOutArgElemTypes[i]);
+    Args.push_back(InOutArgs[i]);
+  }
+  Constraints += InOutConstraints;
+
   // Labels
   SmallVector<llvm::BasicBlock *, 16> Transfer;
   llvm::BasicBlock *Fallthrough = nullptr;
@@ -2565,14 +2573,6 @@ void CodeGenFunction::EmitAsmStmt(const AsmStmt &S) {
     }
   }
 
-  // Append the "input" part of inout constraints last.
-  for (unsigned i = 0, e = InOutArgs.size(); i != e; i++) {
-    ArgTypes.push_back(InOutArgTypes[i]);
-    ArgElemTypes.push_back(InOutArgElemTypes[i]);
-    Args.push_back(InOutArgs[i]);
-  }
-  Constraints += InOutConstraints;
-
   bool HasUnwindClobber = false;
 
   // Clobbers

clang/test/CodeGen/asm-goto.c

@@ -55,14 +55,14 @@ label_true:
 
 int test4(int out1, int out2) {
   // CHECK-LABEL: define{{.*}} i32 @test4(
-  // CHECK: callbr { i32, i32 } asm sideeffect "jne ${3:l}", "={si},={di},r,i,i,0,1
+  // CHECK: callbr { i32, i32 } asm sideeffect "jne ${5:l}", "={si},={di},r,0,1,i,i
   // CHECK: to label %asm.fallthrough [label %label_true, label %loop]
   // CHECK-LABEL: asm.fallthrough:
   if (out1 < out2)
-    asm volatile goto("jne %l3" : "+S"(out1), "+D"(out2) : "r"(out1) :: label_true, loop);
+    asm volatile goto("jne %l5" : "+S"(out1), "+D"(out2) : "r"(out1) :: label_true, loop);
   else
-    asm volatile goto("jne %l5" : "+S"(out1), "+D"(out2) : "r"(out1), "r"(out2) :: label_true, loop);
-  // CHECK: callbr { i32, i32 } asm sideeffect "jne ${5:l}", "={si},={di},r,r,i,i,0,1
+    asm volatile goto("jne %l7" : "+S"(out1), "+D"(out2) : "r"(out1), "r"(out2) :: label_true, loop);
+  // CHECK: callbr { i32, i32 } asm sideeffect "jne ${7:l}", "={si},={di},r,r,0,1,i,i
   // CHECK: to label %asm.fallthrough2 [label %label_true, label %loop]
   // CHECK-LABEL: asm.fallthrough2:
   return out1 + out2;
@@ -74,7 +74,7 @@ label_true:
 
 int test5(int addr, int size, int limit) {
   // CHECK-LABEL: define{{.*}} i32 @test5(
-  // CHECK: callbr i32 asm "add $1,$0 ; jc ${3:l} ; cmp $2,$0 ; ja ${3:l} ; ", "=r,imr,imr,i,0
+  // CHECK: callbr i32 asm "add $1,$0 ; jc ${4:l} ; cmp $2,$0 ; ja ${4:l} ; ", "=r,imr,imr,0,i
   // CHECK: to label %asm.fallthrough [label %t_err]
   // CHECK-LABEL: asm.fallthrough:
   asm goto(
@@ -92,14 +92,40 @@ t_err:
 
 int test6(int out1) {
   // CHECK-LABEL: define{{.*}} i32 @test6(
-  // CHECK: callbr i32 asm sideeffect "testl $0, $0; testl $1, $1; jne ${2:l}", "={si},r,i,i,0,{{.*}} i8* blockaddress(@test6, %label_true), i8* blockaddress(@test6, %landing)
+  // CHECK: callbr i32 asm sideeffect "testl $0, $0; testl $1, $1; jne ${3:l}", "={si},r,0,i,i,{{.*}} i8* blockaddress(@test6, %label_true), i8* blockaddress(@test6, %landing)
   // CHECK: to label %asm.fallthrough [label %label_true, label %landing]
   // CHECK-LABEL: asm.fallthrough:
   // CHECK-LABEL: landing:
   int out2 = 42;
-  asm volatile goto("testl %0, %0; testl %1, %1; jne %l2" : "+S"(out2) : "r"(out1) :: label_true, landing);
+  asm volatile goto("testl %0, %0; testl %1, %1; jne %l3" : "+S"(out2) : "r"(out1) :: label_true, landing);
 landing:
   return out1 + out2;
 label_true:
   return -2;
 }
+
+// test7 - For the output templates in the asm string (%0, %l2), GCC places
+// hidden inputs tied to outputs ("+r" constraint) BEFORE labels. Test that foo
+// is $2 (or rather ${2:l} because of the l output template) in the emitted asm
+// string, not $1.
+void *test7(void) {
+  // CHECK-LABEL: define{{.*}} i8* @test7(
+  // CHECK: %1 = callbr i8* asm "# $0\0A\09# ${2:l}", "=r,0,i,~{dirflag},~{fpsr},~{flags}"(i8* %0, i8* blockaddress(@test7, %foo))
+  // CHECK-NEXT: to label %asm.fallthrough [label %foo]
+  void *p = &&foo;
+  asm goto ("# %0\n\t# %l2":"+r"(p):::foo);
+foo:
+  return p;
+}
+
+// test8 - the same as test7, but this time we use symbolic names rather than
+// numbered outputs.
+void *test8(void) {
+  // CHECK-LABEL: define{{.*}} i8* @test8(
+  // CHECK: %1 = callbr i8* asm "# $0\0A\09# ${2:l}", "=r,0,i,~{dirflag},~{fpsr},~{flags}"(i8* %0, i8* blockaddress(@test8, %foo))
+  // CHECK-NEXT: to label %asm.fallthrough [label %foo]
+  void *p = &&foo;
+  asm goto ("# %0\n\t# %l[foo]":"+r"(p):::foo);
+foo:
+  return p;
+}