Produce a better diagnostic for global register variables.

Currently, when there is a global register variable in a program that
is bound to an invalid register, clang/llvm prints an error message that
is not very user-friendly.

This commit improves the diagnostic and moves the check that used to be
in the backend to Sema. In addition, it makes changes to error out if
the size of the register doesn't match the declared variable size.

e.g., volatile register int B asm ("rbp");

rdar://problem/23084219

Differential Revision: http://reviews.llvm.org/D13834

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@253405 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 3a24282187d07b744698c96901b94933f313f0dd..995bd157aaa93baff0c8c6e9b663e3fd6eb2d657 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6543,6 +6543,10 @@ let CategoryName = "Inline Assembly Issue" in {
     "asm constraint has an unexpected number of alternatives: %0 vs %1">;
   def err_asm_incomplete_type : Error<"asm operand has incomplete type %0">;
   def err_asm_unknown_register_name : Error<"unknown register name '%0' in asm">;
+  def err_asm_invalid_global_var_reg : Error<"register '%0' unsuitable for "
+    "global register variables on this target">;
+  def err_asm_register_size_mismatch : Error<"size of register '%0' does not "
+    "match variable size">;
   def err_asm_bad_register_type : Error<"bad type for named register variable">;
   def err_asm_invalid_input_size : Error<
     "invalid input size for constraint '%0'">;

diff --git a/include/clang/Basic/TargetInfo.h b/include/clang/Basic/TargetInfo.h
index b0c83498c277abce48348301ebcd980b91f0372f..f1d833817077804af614492bd4956a17ee496a75 100644 (file)
--- a/include/clang/Basic/TargetInfo.h
+++ b/include/clang/Basic/TargetInfo.h
@@ -644,6 +644,19 @@ public:
     }
   };
 
+  /// \brief Validate register name used for global register variables.
+  ///
+  /// This function returns true if the register passed in RegName can be used
+  /// for global register variables on this target. In addition, it returns
+  /// true in HasSizeMismatch if the size of the register doesn't match the
+  /// variable size passed in RegSize.
+  virtual bool validateGlobalRegisterVariable(StringRef RegName,
+                                              unsigned RegSize,
+                                              bool &HasSizeMismatch) const {
+    HasSizeMismatch = false;
+    return true;
+  }
+
   // validateOutputConstraint, validateInputConstraint - Checks that
   // a constraint is valid and provides information about it.
   // FIXME: These should return a real error instead of just true/false.

diff --git a/lib/Basic/Targets.cpp b/lib/Basic/Targets.cpp
index 319a8d93116344a2fb3dbf8af29fa3d1f34aa326..648ef4b3d2640cd8587d5d0d54acc797ba2b4f12 100644 (file)
--- a/lib/Basic/Targets.cpp
+++ b/lib/Basic/Targets.cpp
@@ -2358,6 +2358,20 @@ public:
   bool validateAsmConstraint(const char *&Name,
                              TargetInfo::ConstraintInfo &info) const override;
 
+  bool validateGlobalRegisterVariable(StringRef RegName,
+                                      unsigned RegSize,
+                                      bool &HasSizeMismatch) const override {
+    // esp and ebp are the only 32-bit registers the x86 backend can currently
+    // handle.
+    if (RegName.equals("esp") || RegName.equals("ebp")) {
+      // Check that the register size is 32-bit.
+      HasSizeMismatch = RegSize != 32;
+      return true;
+    }
+
+    return false;
+  }
+
   bool validateOutputSize(StringRef Constraint, unsigned Size) const override;
 
   bool validateInputSize(StringRef Constraint, unsigned Size) const override;
@@ -3974,6 +3988,22 @@ public:
 
   // for x32 we need it here explicitly
   bool hasInt128Type() const override { return true; }
+
+  bool validateGlobalRegisterVariable(StringRef RegName,
+                                      unsigned RegSize,
+                                      bool &HasSizeMismatch) const override {
+    // rsp and rbp are the only 64-bit registers the x86 backend can currently
+    // handle.
+    if (RegName.equals("rsp") || RegName.equals("rbp")) {
+      // Check that the register size is 64-bit.
+      HasSizeMismatch = RegSize != 64;
+      return true;
+    }
+
+    // Check if the register is a 32-bit register the backend can handle.
+    return X86TargetInfo::validateGlobalRegisterVariable(RegName, RegSize,
+                                                         HasSizeMismatch);
+  }
 };
 
 // x86-64 Windows target

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 4e2616ebe20da2fdc057ce722cade896e3e31313..261a28890888261e797542366ab8631e5a05fd37 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -6079,9 +6079,20 @@ Sema::ActOnVariableDeclarator(Scope *S, Declarator &D, DeclContext *DC,
       }
     } else if (SC == SC_Register) {
       // Global Named register
-      if (!Context.getTargetInfo().isValidGCCRegisterName(Label) &&
-          DeclAttrsMatchCUDAMode(getLangOpts(), NewVD))
-        Diag(E->getExprLoc(), diag::err_asm_unknown_register_name) << Label;
+      if (DeclAttrsMatchCUDAMode(getLangOpts(), NewVD)) {
+        const auto &TI = Context.getTargetInfo();
+        bool HasSizeMismatch;
+
+        if (!TI.isValidGCCRegisterName(Label))
+          Diag(E->getExprLoc(), diag::err_asm_unknown_register_name) << Label;
+        else if (!TI.validateGlobalRegisterVariable(Label,
+                                                    Context.getTypeSize(R),
+                                                    HasSizeMismatch))
+          Diag(E->getExprLoc(), diag::err_asm_invalid_global_var_reg) << Label;
+        else if (HasSizeMismatch)
+          Diag(E->getExprLoc(), diag::err_asm_register_size_mismatch) << Label;
+      }
+
       if (!R->isIntegralType(Context) && !R->isPointerType()) {
         Diag(D.getLocStart(), diag::err_asm_bad_register_type);
         NewVD->setInvalidDecl(true);

diff --git a/test/CodeGen/named_reg_global.c b/test/CodeGen/named_reg_global.c
index 8f9a9c685d1fc9e5f3f63ecc48832a96efd45065..1da62574689137fdc93415417695cd5b93ab4579 100644 (file)
--- a/test/CodeGen/named_reg_global.c
+++ b/test/CodeGen/named_reg_global.c
@@ -1,16 +1,26 @@
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple arm64-linux-gnu -S -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple armv7-linux-gnu -S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-X86-64
+// RUN: %clang_cc1 -triple arm64-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ARM
+// RUN: %clang_cc1 -triple armv7-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ARM
 
 // CHECK-NOT: @sp = common global
+
+#if defined(__x86_64__)
+register unsigned long current_stack_pointer asm("rsp");
+#else
 register unsigned long current_stack_pointer asm("sp");
+#endif
+
 struct p4_Thread {
   struct {
     int len;
   } word;
 };
 // Testing pointer types as well
+#if defined(__x86_64__)
+register struct p4_Thread *p4TH asm("rsp");
+#else
 register struct p4_Thread *p4TH asm("sp");
+#endif
 
 // CHECK: define{{.*}} i[[bits:[0-9]+]] @get_stack_pointer_addr()
 // CHECK: [[ret:%[0-9]+]] = call i[[bits]] @llvm.read_register.i[[bits]](metadata !0)
@@ -43,5 +53,7 @@ void fn2(struct p4_Thread *val) {
 // CHECK: %[[regw:[0-9]+]] = ptrtoint %struct.p4_Thread* %{{.*}} to i[[bits]]
 // CHECK: call void @llvm.write_register.i[[bits]](metadata !0, i[[bits]] %[[regw]])
 
-// CHECK: !llvm.named.register.sp = !{!0}
-// CHECK: !0 = !{!"sp"}
+// CHECK-X86-64: !llvm.named.register.rsp = !{!0}
+// CHECK-X86-64: !0 = !{!"rsp"}
+// CHECK-ARM: !llvm.named.register.sp = !{!0}
+// CHECK-ARM: !0 = !{!"sp"}

diff --git a/test/OpenMP/atomic_capture_codegen.cpp b/test/OpenMP/atomic_capture_codegen.cpp
index 6dd9f7ab2a41f2c2d32ced7af18d1b3f24032f2f..f91da3e80f542306b64d24aec470c5a00caeaa79 100644 (file)
--- a/test/OpenMP/atomic_capture_codegen.cpp
+++ b/test/OpenMP/atomic_capture_codegen.cpp
@@ -72,7 +72,10 @@ struct BitFields4_packed {
 typedef float float2 __attribute__((ext_vector_type(2)));
 float2 float2x;
 
-register int rix __asm__("0");
+// Register "0" is currently an invalid register for global register variables.
+// Use "esp" instead of "0".
+// register int rix __asm__("0");
+register int rix __asm__("esp");
 
 int main() {
 // CHECK: [[PREV:%.+]] = atomicrmw add i8* @{{.+}}, i8 1 monotonic

diff --git a/test/OpenMP/atomic_read_codegen.c b/test/OpenMP/atomic_read_codegen.c
index fc47c82d891309b97ca28ab63864ca0c2fd8600c..0cd46e3821fd8d5681fbc84cb90257914f91fcb5 100644 (file)
--- a/test/OpenMP/atomic_read_codegen.c
+++ b/test/OpenMP/atomic_read_codegen.c
@@ -72,7 +72,10 @@ struct BitFields4_packed {
 typedef float float2 __attribute__((ext_vector_type(2)));
 float2 float2x;
 
-register int rix __asm__("0");
+// Register "0" is currently an invalid register for global register variables.
+// Use "esp" instead of "0".
+// register int rix __asm__("0");
+register int rix __asm__("esp");
 
 int main() {
 // CHECK: load atomic i8, i8*

diff --git a/test/OpenMP/atomic_update_codegen.cpp b/test/OpenMP/atomic_update_codegen.cpp
index df8b538ee165bc679d2aaef3f1ea96306bb606c4..063b76d7ff54b94d37ab595ce5b4fcb12aa09553 100644 (file)
--- a/test/OpenMP/atomic_update_codegen.cpp
+++ b/test/OpenMP/atomic_update_codegen.cpp
@@ -72,7 +72,10 @@ struct BitFields4_packed {
 typedef float float2 __attribute__((ext_vector_type(2)));
 float2 float2x;
 
-register int rix __asm__("0");
+// Register "0" is currently an invalid register for global register variables.
+// Use "esp" instead of "0".
+// register int rix __asm__("0");
+register int rix __asm__("esp");
 
 int main() {
 // CHECK-NOT: atomicrmw

diff --git a/test/OpenMP/atomic_write_codegen.c b/test/OpenMP/atomic_write_codegen.c
index 1ee26b078212a8bcf496818affd2b686cc93262b..66172af07e8f4671e0322b106dcafae4bf792e76 100644 (file)
--- a/test/OpenMP/atomic_write_codegen.c
+++ b/test/OpenMP/atomic_write_codegen.c
@@ -72,7 +72,10 @@ struct BitFields4_packed {
 typedef float float2 __attribute__((ext_vector_type(2)));
 float2 float2x;
 
-register int rix __asm__("0");
+// Register "0" is currently an invalid register for global register variables.
+// Use "esp" instead of "0".
+// register int rix __asm__("0");
+register int rix __asm__("esp");
 
 int main() {
 // CHECK: load i8, i8*

diff --git a/test/OpenMP/for_loop_messages.cpp b/test/OpenMP/for_loop_messages.cpp
index 14b20aeb9a3e27e7164ce7e4b04fff7c115e432b..7f0b11410102974d44928a4fad5f39b9e16f9bc7 100644 (file)
--- a/test/OpenMP/for_loop_messages.cpp
+++ b/test/OpenMP/for_loop_messages.cpp
@@ -13,7 +13,9 @@ static int sii;
 #pragma omp threadprivate(sii)
 static int globalii;
 
-register int reg0 __asm__("0");
+// Currently, we cannot use "0" for global register variables.
+// register int reg0 __asm__("0");
+int reg0;
 
 int test_iteration_spaces() {
   const int N = 100;

diff --git a/test/OpenMP/threadprivate_messages.cpp b/test/OpenMP/threadprivate_messages.cpp
index 39a4431f0c5bf913cd3d28499276c6744628b2ca..f71d58bc52a4deb348b16085039d4377d0d5c7ea 100644 (file)
--- a/test/OpenMP/threadprivate_messages.cpp
+++ b/test/OpenMP/threadprivate_messages.cpp
@@ -96,7 +96,10 @@ class TempClass {
 static __thread int t; // expected-note {{'t' defined here}}
 #pragma omp threadprivate (t) // expected-error {{variable 't' cannot be threadprivate because it is thread-local}}
 
-register int reg0 __asm__("0"); // expected-note {{'reg0' defined here}}
+// Register "0" is currently an invalid register for global register variables.
+// Use "esp" instead of "0".
+// register int reg0 __asm__("0");
+register int reg0 __asm__("esp"); // expected-note {{'reg0' defined here}}
 #pragma omp threadprivate (reg0) // expected-error {{variable 'reg0' cannot be threadprivate because it is a global named register variable}}
 
 int o; // expected-note {{candidate found by name lookup is 'o'}}

diff --git a/test/Sema/asm.c b/test/Sema/asm.c
index 76e20158c403ed39f183ae76495841d8a034fb6b..d29b136a117ac5251cf5ce33ff65439c17130616 100644 (file)
--- a/test/Sema/asm.c
+++ b/test/Sema/asm.c
@@ -1,7 +1,6 @@
 // RUN: %clang_cc1 %s -Wno-private-extern -triple i386-pc-linux-gnu -verify -fsyntax-only
 
 
-
 void f() {
   int i;
 
@@ -154,10 +153,13 @@ double test15() {
 // PR19837
 struct foo {
   int a;
-  char b;
 };
-register struct foo bar asm("sp"); // expected-error {{bad type for named register variable}}
-register float baz asm("sp"); // expected-error {{bad type for named register variable}}
+register struct foo bar asm("esp"); // expected-error {{bad type for named register variable}}
+register float baz asm("esp"); // expected-error {{bad type for named register variable}}
+
+register int r0 asm ("edi"); // expected-error {{register 'edi' unsuitable for global register variables on this target}}
+register long long r1 asm ("esp"); // expected-error {{size of register 'esp' does not match variable size}}
+register int r2 asm ("esp");
 
 double f_output_constraint(void) {
   double result;
@@ -212,7 +214,7 @@ typedef struct test16_foo {
   unsigned int field3 : 3;
 } test16_foo;
 typedef __attribute__((vector_size(16))) int test16_bar;
-register int test16_baz asm("rbx");
+register int test16_baz asm("esp");
 
 void test16()
 {

diff --git a/test/SemaCUDA/asm-constraints-mixed.cu b/test/SemaCUDA/asm-constraints-mixed.cu
index ebf44243d9d68b3047cb3251a8dbe5ae4c4c6821..a3b1e1a08c5101819806fe65ea13c88313d17def 100644 (file)
--- a/test/SemaCUDA/asm-constraints-mixed.cu
+++ b/test/SemaCUDA/asm-constraints-mixed.cu
@@ -5,14 +5,14 @@
 
 __attribute__((device)) register long global_dev_reg asm("r0");
 __attribute__((device)) register long
-    global_dev_hreg asm("rax"); // device-side error
+    global_dev_hreg asm("rsp"); // device-side error
 
-register long global_host_reg asm("rax");
+register long global_host_reg asm("rsp");
 register long global_host_dreg asm("r0"); // host-side error
 
 __attribute__((device)) void df() {
   register long local_dev_reg asm("r0");
-  register long local_host_reg asm("rax"); // device-side error
+  register long local_host_reg asm("rsp"); // device-side error
   short h;
   // asm with PTX constraints. Some of them are PTX-specific.
   __asm__("dont care" : "=h"(h) : "f"(0.0), "d"(0.0), "h"(0), "r"(0), "l"(0));
@@ -20,7 +20,7 @@ __attribute__((device)) void df() {
 
 void hf() {
   register long local_dev_reg asm("r0"); // host-side error
-  register long local_host_reg asm("rax");
+  register long local_host_reg asm("rsp");
   int a;
   // Asm with x86 constraints and registers that are not supported by PTX.
   __asm__("dont care" : "=a"(a) : "a"(0), "b"(0), "c"(0) : "flags");
@@ -30,8 +30,8 @@ void hf() {
 // We should only see errors relevant to current compilation mode.
 #if defined(__CUDA_ARCH__)
 // Device-side compilation:
-// expected-error@8 {{unknown register name 'rax' in asm}}
-// expected-error@15 {{unknown register name 'rax' in asm}}
+// expected-error@8 {{unknown register name 'rsp' in asm}}
+// expected-error@15 {{unknown register name 'rsp' in asm}}
 #else
 // Host-side compilation:
 // expected-error@11 {{unknown register name 'r0' in asm}}