[clang] 99c0a3e - [RISCV] Enable target attribute when invoked through clang driver (#74889)

[via cfe-commits]

Author: Philip Reames
Date: 2023-12-11T08:55:21-08:00
New Revision: 99c0a3ea98724798361c8ef72d07d9646dcb64ee

URL: https://github.com/llvm/llvm-project/commit/99c0a3ea98724798361c8ef72d07d9646dcb64ee
DIFF: https://github.com/llvm/llvm-project/commit/99c0a3ea98724798361c8ef72d07d9646dcb64ee.diff

LOG: [RISCV] Enable target attribute when invoked through clang driver (#74889)

d80e46d added support for the target function attribute. However, it
turns out that commit has a nasty bug/oversight. As the tests in that
revision show, everything works if clang -cc1 is directly invoked. I was
suprised to learn this morning that compiling with clang (i.e. the
typical user workflow) did not work.

The bug is that if a set of explicit negative extensions is passed to
cc1 at the command line (as the clang driver always does), we were
copying these negative extensions to the end of the rewritten extension
list. When this was later parsed, this had the effect of turning back
off any extension that the target attribute had enabled.

This patch updates the logic to only propagate the features from the
input which don't appear in the rewritten form in either positive or
negative form.

Note that this code structure is still highly suspect. In particular I'm
fairly sure that mixing extension versions with this code will result in
odd results. However, I figure its better to have something which mostly
works than something which doesn't work at all.

Added: 
    

Modified: 
    clang/lib/Basic/Targets/RISCV.cpp
    clang/test/CodeGen/RISCV/riscv-func-attr-target.c

Removed: 
    


################################################################################
diff  --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index b1733b878e88ed..45d23022b5306b 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -304,11 +304,18 @@ bool RISCVTargetInfo::initFeatureMap(
 
   // RISCVISAInfo makes implications for ISA features
   std::vector<std::string> ImpliedFeatures = (*ParseResult)->toFeatureVector();
-  // Add non-ISA features like `relax` and `save-restore` back
-  for (const std::string &Feature : NewFeaturesVec)
-    if (!llvm::is_contained(ImpliedFeatures, Feature))
-      ImpliedFeatures.push_back(Feature);
 
+  // parseFeatures normalizes the feature set by dropping any explicit
+  // negatives, and non-extension features.  We need to preserve the later
+  // for correctness and want to preserve the former for consistency.
+  for (auto &Feature : NewFeaturesVec) {
+     StringRef ExtName = Feature;
+     assert(ExtName.size() > 1 && (ExtName[0] == '+' || ExtName[0] == '-'));
+     ExtName = ExtName.drop_front(1); // Drop '+' or '-'
+     if (!llvm::is_contained(ImpliedFeatures, ("+" + ExtName).str()) &&
+         !llvm::is_contained(ImpliedFeatures, ("-" + ExtName).str()))
+       ImpliedFeatures.push_back(Feature);
+  }
   return TargetInfo::initFeatureMap(Features, Diags, CPU, ImpliedFeatures);
 }
 

diff  --git a/clang/test/CodeGen/RISCV/riscv-func-attr-target.c b/clang/test/CodeGen/RISCV/riscv-func-attr-target.c
index 74bc5f2ac70492..506acaba687417 100644
--- a/clang/test/CodeGen/RISCV/riscv-func-attr-target.c
+++ b/clang/test/CodeGen/RISCV/riscv-func-attr-target.c
@@ -1,6 +1,7 @@
 // REQUIRES: riscv-registered-target
 // RUN: %clang_cc1 -triple riscv64 -target-feature +zifencei -target-feature +m \
-// RUN:  -target-feature +a -target-feature +save-restore \
+// RUN:  -target-feature +a -target-feature +save-restore -target-feature -zbb \
+// RUN:  -target-feature -relax -target-feature -zfa \
 // RUN:  -emit-llvm %s -o - | FileCheck %s
 
 // CHECK-LABEL: define dso_local void @testDefault
@@ -35,12 +36,12 @@ testAttrFullArchAndAttrCpu() {}
 __attribute__((target("cpu=sifive-u54"))) void testAttrCpuOnly() {}
 
 //.
-// CHECK: attributes #0 = { {{.*}}"target-features"="+64bit,+a,+m,+save-restore,+zifencei" }
-// CHECK: attributes #1 = { {{.*}}"target-cpu"="rocket-rv64" "target-features"="+64bit,+a,+d,+f,+m,+save-restore,+v,+zicsr,+zifencei,+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b" "tune-cpu"="generic-rv64" }
-// CHECK: attributes #2 = { {{.*}}"target-features"="+64bit,+a,+m,+save-restore,+zbb,+zifencei" }
-// CHECK: attributes #3 = { {{.*}}"target-features"="+64bit,+a,+d,+experimental-zicond,+f,+m,+save-restore,+v,+zbb,+zicsr,+zifencei,+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b" }
-// CHECK: attributes #4 = { {{.*}}"target-features"="+64bit,+a,+c,+d,+f,+m,+save-restore,+zbb,+zicsr,+zifencei" }
-// CHECK: attributes #5 = { {{.*}}"target-features"="+64bit,+m,+save-restore" }
-// CHECK: attributes #6 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+a,+m,+save-restore,+zbb,+zifencei" }
-// CHECK: attributes #7 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+m,+save-restore" }
-// CHECK: attributes #8 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+a,+c,+d,+f,+m,+save-restore,+zicsr,+zifencei" }
+// CHECK: attributes #0 = { {{.*}}"target-features"="+64bit,+a,+m,+save-restore,+zifencei,-relax,-zbb,-zfa" }
+// CHECK: attributes #1 = { {{.*}}"target-cpu"="rocket-rv64" "target-features"="+64bit,+a,+d,+f,+m,+save-restore,+v,+zicsr,+zifencei,+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b,-relax,-zbb,-zfa" "tune-cpu"="generic-rv64" }
+// CHECK: attributes #2 = { {{.*}}"target-features"="+64bit,+a,+m,+save-restore,+zbb,+zifencei,-relax,-zfa" }
+// CHECK: attributes #3 = { {{.*}}"target-features"="+64bit,+a,+d,+experimental-zicond,+f,+m,+save-restore,+v,+zbb,+zicsr,+zifencei,+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b,-relax,-zfa" }
+// CHECK: attributes #4 = { {{.*}}"target-features"="+64bit,+a,+c,+d,+f,+m,+save-restore,+zbb,+zicsr,+zifencei,-relax,-zfa" }
+// CHECK: attributes #5 = { {{.*}}"target-features"="+64bit,+m,+save-restore,-relax,-zbb,-zfa" }
+// CHECK: attributes #6 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+a,+m,+save-restore,+zbb,+zifencei,-relax,-zfa" }
+// CHECK: attributes #7 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+m,+save-restore,-relax,-zbb,-zfa" }
+// CHECK: attributes #8 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+a,+c,+d,+f,+m,+save-restore,+zicsr,+zifencei,-relax,-zbb,-zfa" }