[clang] [Clang][RISCV] Recognize unsupport target feature by supporting isValidFeatureName (PR #106495)

[Piyou Chen via cfe-commits]

https://github.com/BeMg updated https://github.com/llvm/llvm-project/pull/106495

>From e8f472674e0d1e70adcd1d29b8c902f4cd80f188 Mon Sep 17 00:00:00 2001
From: Piyou Chen <piyou.chen at sifive.com>
Date: Wed, 28 Aug 2024 21:15:57 -0700
Subject: [PATCH 1/3] [Clang][RISCV] Recognize unsupport feature by supporting
 isValidFeatureName

---
 clang/lib/Basic/Targets/RISCV.cpp   |  7 +++++++
 clang/lib/Basic/Targets/RISCV.h     |  1 +
 clang/lib/Sema/SemaDeclAttr.cpp     | 15 +++++++++++----
 clang/test/Sema/attr-target-riscv.c |  9 +++++++++
 4 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index b89109e7725d44..9abef57d007468 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -478,3 +478,10 @@ bool RISCVTargetInfo::validateCpuSupports(StringRef Feature) const {
   // __riscv_feature_bits structure.
   return -1 != llvm::RISCVISAInfo::getRISCVFeaturesBitsInfo(Feature).second;
 }
+
+bool RISCVTargetInfo::isValidFeatureName(StringRef Name) const {
+  if (Name == "__RISCV_TargetAttrNeedOverride")
+    return true;
+
+  return llvm::RISCVISAInfo::isSupportedExtensionFeature(Name);
+}
diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
index 626274b8fc437c..b808ccc8e9cfe9 100644
--- a/clang/lib/Basic/Targets/RISCV.h
+++ b/clang/lib/Basic/Targets/RISCV.h
@@ -130,6 +130,7 @@ class RISCVTargetInfo : public TargetInfo {
   bool supportsCpuSupports() const override { return getTriple().isOSLinux(); }
   bool supportsCpuInit() const override { return getTriple().isOSLinux(); }
   bool validateCpuSupports(StringRef Feature) const override;
+  bool isValidFeatureName(StringRef Name) const override;
 };
 class LLVM_LIBRARY_VISIBILITY RISCV32TargetInfo : public RISCVTargetInfo {
 public:
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 1e074298ac5289..81cff8d7362ad5 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -2993,10 +2993,17 @@ bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
     return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
            << Unknown << Tune << ParsedAttrs.Tune << Target;
 
-  if (Context.getTargetInfo().getTriple().isRISCV() &&
-      ParsedAttrs.Duplicate != "")
-    return Diag(LiteralLoc, diag::err_duplicate_target_attribute)
-           << Duplicate << None << ParsedAttrs.Duplicate << Target;
+  if (Context.getTargetInfo().getTriple().isRISCV()) {
+    if (ParsedAttrs.Duplicate != "")
+      return Diag(LiteralLoc, diag::err_duplicate_target_attribute)
+             << Duplicate << None << ParsedAttrs.Duplicate << Target;
+    for (const auto &Feature : ParsedAttrs.Features) {
+      auto CurFeature = StringRef(Feature);
+      if (!CurFeature.starts_with("+") && !CurFeature.starts_with("-"))
+        return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
+               << Unsupported << None << AttrStr << Target;
+    }
+  }
 
   if (ParsedAttrs.Duplicate != "")
     return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
diff --git a/clang/test/Sema/attr-target-riscv.c b/clang/test/Sema/attr-target-riscv.c
index ed4e2915d6c6ef..01d928c1d78e48 100644
--- a/clang/test/Sema/attr-target-riscv.c
+++ b/clang/test/Sema/attr-target-riscv.c
@@ -4,3 +4,12 @@
 int __attribute__((target("arch=rv64g"))) foo(void) { return 0; }
 //expected-error at +1 {{redefinition of 'foo'}}
 int __attribute__((target("arch=rv64gc"))) foo(void) { return 0; }
+
+//expected-warning at +1 {{unsupported 'notafeature' in the 'target' attribute string; 'target' attribute ignored}}
+int __attribute__((target("arch=+notafeature"))) UnsupportFeature(void) { return 0; }
+
+//expected-warning at +1 {{unsupported 'arch=+zba,zbb' in the 'target' attribute string; 'target' attribute ignored}}
+int __attribute__((target("arch=+zba,zbb"))) WithoutAddSigned(void) { return 0; }
+
+//expected-warning at +1 {{unsupported 'arch=zba' in the 'target' attribute string; 'target' attribute ignored}}
+int __attribute__((target("arch=zba"))) WithoutAddSigned2(void) { return 0; }

>From ad95a3112c5349ea125d9c8d79cfa2f5e3d1d0a4 Mon Sep 17 00:00:00 2001
From: Piyou Chen <piyou.chen at sifive.com>
Date: Thu, 29 Aug 2024 00:39:08 -0700
Subject: [PATCH 2/3] Merge handleFullArchString logic, parseArchString will
 check whether start with "rv"

---
 clang/lib/Basic/Targets/RISCV.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index 9abef57d007468..1ab9f19ab7a30c 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -388,7 +388,7 @@ static void handleFullArchString(StringRef FullArchStr,
       FullArchStr, /* EnableExperimentalExtension */ true);
   if (llvm::errorToBool(RII.takeError())) {
     // Forward the invalid FullArchStr.
-    Features.push_back("+" + FullArchStr.str());
+    Features.push_back(FullArchStr.str());
   } else {
     // Append a full list of features, including any negative extensions so that
     // we override the CPU's features.

>From 81d79a5bb2a8b65f70465288841dc484d960c90a Mon Sep 17 00:00:00 2001
From: Piyou Chen <piyou.chen at sifive.com>
Date: Thu, 29 Aug 2024 22:06:28 -0700
Subject: [PATCH 3/3] use '+'/'-'

---
 clang/lib/Sema/SemaDeclAttr.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 81cff8d7362ad5..a76ad6b8b096bf 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -2999,7 +2999,7 @@ bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
              << Duplicate << None << ParsedAttrs.Duplicate << Target;
     for (const auto &Feature : ParsedAttrs.Features) {
       auto CurFeature = StringRef(Feature);
-      if (!CurFeature.starts_with("+") && !CurFeature.starts_with("-"))
+      if (!CurFeature.starts_with('+') && !CurFeature.starts_with('-'))
         return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
                << Unsupported << None << AttrStr << Target;
     }