ahatanak created this revision. ahatanak added a subscriber: cfe-commits. clang doesn't print a very user-friendly message when an invalid register is used for a global register variable:
For example, when the following code is compiled, $ cat f1.c volatile register long long A asm ("rdi"); void foo1() { A = 1; } clang prints this error message: $ clang -c f1.c fatal error: error in backend: Invalid register name global variable The code fails to compile because "rdi" isn't a valid register for global register variables on x86 (rsp, rbp, esp, and ebp are the only registers that are currently valid), but the diagnostic doesn't give much detail on why it is an error or which line of the source code is not correct because the error is detected in the backend. This patch makes changes in Sema to catch this kind of error earlier. In addition, it errors out if the size of the register doesn't match the declared variable size. e.g., volatile register int B asm ("rbp"); http://reviews.llvm.org/D13834 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Basic/TargetInfo.h lib/Basic/Targets.cpp lib/Sema/SemaDecl.cpp test/CodeGen/named_reg_global.c test/OpenMP/atomic_capture_codegen.cpp test/OpenMP/atomic_read_codegen.c test/OpenMP/atomic_update_codegen.cpp test/OpenMP/atomic_write_codegen.c test/OpenMP/for_loop_messages.cpp test/OpenMP/threadprivate_messages.cpp test/Sema/asm.c test/SemaCUDA/asm-constraints-mixed.cu
Index: test/SemaCUDA/asm-constraints-mixed.cu =================================================================== --- test/SemaCUDA/asm-constraints-mixed.cu +++ test/SemaCUDA/asm-constraints-mixed.cu @@ -5,22 +5,22 @@ __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)); } 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 @@ // 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}} Index: test/Sema/asm.c =================================================================== --- test/Sema/asm.c +++ 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 @@ // 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 @@ 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() { Index: test/OpenMP/threadprivate_messages.cpp =================================================================== --- test/OpenMP/threadprivate_messages.cpp +++ test/OpenMP/threadprivate_messages.cpp @@ -96,7 +96,10 @@ 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'}} Index: test/OpenMP/for_loop_messages.cpp =================================================================== --- test/OpenMP/for_loop_messages.cpp +++ test/OpenMP/for_loop_messages.cpp @@ -13,7 +13,9 @@ #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; Index: test/OpenMP/atomic_write_codegen.c =================================================================== --- test/OpenMP/atomic_write_codegen.c +++ test/OpenMP/atomic_write_codegen.c @@ -72,7 +72,10 @@ 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* Index: test/OpenMP/atomic_update_codegen.cpp =================================================================== --- test/OpenMP/atomic_update_codegen.cpp +++ test/OpenMP/atomic_update_codegen.cpp @@ -72,7 +72,10 @@ 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 Index: test/OpenMP/atomic_read_codegen.c =================================================================== --- test/OpenMP/atomic_read_codegen.c +++ test/OpenMP/atomic_read_codegen.c @@ -72,7 +72,10 @@ 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* Index: test/OpenMP/atomic_capture_codegen.cpp =================================================================== --- test/OpenMP/atomic_capture_codegen.cpp +++ test/OpenMP/atomic_capture_codegen.cpp @@ -72,7 +72,10 @@ 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 Index: test/CodeGen/named_reg_global.c =================================================================== --- test/CodeGen/named_reg_global.c +++ 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 @@ // 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"} Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -6035,9 +6035,20 @@ } } 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); Index: lib/Basic/Targets.cpp =================================================================== --- lib/Basic/Targets.cpp +++ lib/Basic/Targets.cpp @@ -2375,6 +2375,17 @@ bool validateAsmConstraint(const char *&Name, TargetInfo::ConstraintInfo &info) const override; + bool validateGlobalRegisterVariable(StringRef RegName, + unsigned RegSize, + bool &HasSizeMismatch) const override { + if (RegName.equals("esp") || RegName.equals("ebp")) { + HasSizeMismatch = RegSize != 32; + return true; + } + + return false; + } + bool validateOutputSize(StringRef Constraint, unsigned Size) const override; bool validateInputSize(StringRef Constraint, unsigned Size) const override; @@ -3960,6 +3971,18 @@ // for x32 we need it here explicitly bool hasInt128Type() const override { return true; } + + bool validateGlobalRegisterVariable(StringRef RegName, + unsigned RegSize, + bool &HasSizeMismatch) const override { + if (RegName.equals("rsp") || RegName.equals("rbp")) { + HasSizeMismatch = RegSize != 64; + return true; + } + + return X86TargetInfo::validateGlobalRegisterVariable(RegName, RegSize, + HasSizeMismatch); + } }; // x86-64 Windows target Index: include/clang/Basic/TargetInfo.h =================================================================== --- include/clang/Basic/TargetInfo.h +++ include/clang/Basic/TargetInfo.h @@ -645,6 +645,19 @@ } }; + /// \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. Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -6420,6 +6420,10 @@ "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'">;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits