[RISCV] Implement validateGlobalRegisterVariable.

[topperc]

Only allow GPR registers and verify the size is the same as XLen.

This fixes the crash seen in #109588 by making it a frontend error.

gcc does accept the code so we may need to consider if we can fix the backend. Some other targets I tried appear to have similar issues so it might not be straightforward to fix.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-risc-v

Author: Craig Topper (topperc)

Changes

Only allow GPR registers and verify the size is the same as XLen.

This fixes the crash seen in #109588 by making it a frontend error.

gcc does accept the code so we may need to consider if we can fix the backend. Some other targets I tried appear to have similar issues so it might not be straightforward to fix.

---

Full diff: https://github.com/llvm/llvm-project/pull/109596.diff

3 Files Affected:

• (modified) clang/lib/Basic/Targets/RISCV.cpp (+12)
• (modified) clang/lib/Basic/Targets/RISCV.h (+3)
• (modified) clang/test/Sema/riscv-asm.c (+15-2)

diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index a4925e84784af9..b6ea4440507ea1 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -486,3 +486,15 @@ bool RISCVTargetInfo::validateCpuSupports(StringRef Feature) const {
 bool RISCVTargetInfo::isValidFeatureName(StringRef Name) const {
   return llvm::RISCVISAInfo::isSupportedExtensionFeature(Name);
 }
+
+bool RISCVTargetInfo::validateGlobalRegisterVariable(
+    StringRef RegName, unsigned RegSize, bool &HasSizeMismatch) const {
+  if (RegName == "ra" || RegName == "sp" || RegName == "gp" ||
+      RegName == "tp" || RegName.starts_with("x") || RegName.starts_with("a") ||
+      RegName.starts_with("s") || RegName.starts_with("t")) {
+    unsigned XLen = getTriple().isArch64Bit() ? 64 : 32;
+    HasSizeMismatch = RegSize != XLen;
+    return true;
+  }
+  return false;
+}
diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
index b808ccc8e9cfe9..351ef21e197c4d 100644
--- a/clang/lib/Basic/Targets/RISCV.h
+++ b/clang/lib/Basic/Targets/RISCV.h
@@ -131,6 +131,9 @@ class RISCVTargetInfo : public TargetInfo {
   bool supportsCpuInit() const override { return getTriple().isOSLinux(); }
   bool validateCpuSupports(StringRef Feature) const override;
   bool isValidFeatureName(StringRef Name) const override;
+
+  bool validateGlobalRegisterVariable(StringRef RegName, unsigned RegSize,
+                                      bool &HasSizeMismatch) const override;
 };
 class LLVM_LIBRARY_VISIBILITY RISCV32TargetInfo : public RISCVTargetInfo {
 public:
diff --git a/clang/test/Sema/riscv-asm.c b/clang/test/Sema/riscv-asm.c
index 82664c013175d4..69ba3be3345d5a 100644
--- a/clang/test/Sema/riscv-asm.c
+++ b/clang/test/Sema/riscv-asm.c
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 %s -triple riscv32 -verify -fsyntax-only
 // RUN: %clang_cc1 %s -triple riscv64 -verify -fsyntax-only
 
-// expected-no-diagnostics
-
 void i (void) {
   asm volatile ("" ::: "x0",  "x1",  "x2",  "x3",  "x4",  "x5",  "x6",  "x7");
   asm volatile ("" ::: "x8",  "x9",  "x10", "x11", "x12", "x13", "x14", "x15");
@@ -26,3 +24,18 @@ void f (void) {
   asm volatile ("" ::: "fa6", "fa7", "fs2",  "fs3",  "fs4", "fs5", "fs6",  "fs7");
   asm volatile ("" ::: "fs8", "fs9", "fs10", "fs11", "ft8", "ft9", "ft10", "ft11");
 }
+
+register char i1 __asm__ ("x1"); // expected-error {{size of register 'x1' does not match variable size}}
+#if __riscv_xlen == 32
+register long long ll2 __asm__ ("x2"); // expected-error {{size of register 'x2' does not match variable size}}
+register int i2 __asm__ ("x3");
+#endif
+register long l3 __asm__ ("x4");
+register long ra __asm__ ("ra");
+register long sp __asm__ ("sp");
+register int *gp __asm__ ("gp");
+register char *tp __asm__ ("tp");
+register long a7 __asm__ ("a7");
+register long s11 __asm__ ("s11");
+register long t5 __asm__ ("t5");
+register long* f1 __asm__ ("f1"); // expected-error {{register 'f1' unsuitable for global register variables on this target}}

[dtcxzyw]

LGTM

[dtcxzyw]

Should we accept zero?

[wangpc-pp]

zero is not writable/allocatable?

[topperc]

Yeah it didn't seem useful. You can still do it with "x0" because I didn't think it was worth the extra code to check for it.