[clang] 5c562f6 - [clang] number labels in asm goto strings after tied inputs
Nick Desaulniers via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 11 12:10:19 PST 2022
Author: Nick Desaulniers
Date: 2022-01-11T12:09:24-08:00
New Revision: 5c562f62a4ee15592f5d764d0710553a4b07a6f2
URL: https://github.com/llvm/llvm-project/commit/5c562f62a4ee15592f5d764d0710553a4b07a6f2
DIFF: https://github.com/llvm/llvm-project/commit/5c562f62a4ee15592f5d764d0710553a4b07a6f2.diff
LOG: [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
Added:
Modified:
clang/docs/LanguageExtensions.rst
clang/docs/ReleaseNotes.rst
clang/lib/AST/Stmt.cpp
clang/lib/CodeGen/CGStmt.cpp
clang/test/CodeGen/asm-goto.c
Removed:
################################################################################
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index a46edf6ca5061..cadd4f570c5ac 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/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
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d7c78c73ceb03..5481a8cdb60d5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/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
diff erent
+ 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
-----------------------------
diff --git a/clang/lib/AST/Stmt.cpp b/clang/lib/AST/Stmt.cpp
index 4f76f6ec12ed5..be19d3b2cce2c 100644
--- a/clang/lib/AST/Stmt.cpp
+++ b/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;
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 0169fc41bc9e3..520483bc08b67 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/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
diff --git a/clang/test/CodeGen/asm-goto.c b/clang/test/CodeGen/asm-goto.c
index 4ec0a12d0d64e..380ba956d9f09 100644
--- a/clang/test/CodeGen/asm-goto.c
+++ b/clang/test/CodeGen/asm-goto.c
@@ -55,14 +55,14 @@ int test3(int out1, int out2) {
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 @@ int test4(int out1, int out2) {
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 @@ int test5(int addr, int size, int limit) {
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;
+}
More information about the cfe-commits
mailing list