Filter out invalid 'target' items from being passed to LLVM

[erichkeane]

Summary

Craig noticed that CodeGen wasn't properly ignoring the
values sent to the target attribute. This patch ignores
them.

This patch also sets the 'default' for this checking to
'supported', since only X86 has implemented the support
for checking valid CPU names and Feature Names.

One test was changed to i686, since it uses a lakemont,
which would otherwise be prohibited in x86_64.

[erichkeane]

lib/Basic/Targets/X86.cpp

1173 ↗

(On Diff #120528)

A test also revealed that we forgot this as well. Craig just added sse4, which was also missing.

[craig.topper]

lib/CodeGen/CodeGenModule.cpp

4586 ↗

(On Diff #120528)

Use llvm::remove_if which takes a type that has a range?

[erichkeane]

Switched to llvm::remove_if(RANGE, lambda) instead of std::remove_if with begin/end per Craig's suggestion.

[craig.topper]

LGTM

cfe/trunk/include/clang/Basic/TargetInfo.h

public:
   ///
   /// \return  False on error (invalid CPU name).
   virtual bool setCPU(const std::string &Name) {
     return false;
   }
 
   /// brief Determine whether this TargetInfo supports the given CPU name.
   virtual bool isValidCPUName(StringRef Name) const {
-    return false;
+    return true;
   }
 
   /// \brief Use the specified ABI.
   ///
   /// \return False on error (invalid ABI name).
   virtual bool setABI(const std::string &Name) {
     return false;
   }
public:
   virtual void setFeatureEnabled(llvm::StringMap<bool> &Features,
                                  StringRef Name,
                                  bool Enabled) const {
     Features[Name] = Enabled;
   }
 
   /// \brief Determine whether this TargetInfo supports the given feature.
   virtual bool isValidFeatureName(StringRef Feature) const {
-    return false;
+    return true;
   }
 
   /// \brief Perform initialization based on the user configured
   /// set of features (e.g., +sse4).
   ///
   /// The list is guaranteed to have at most one entry per feature.
   ///
   /// The target may modify the features list, to change which options are

cfe/trunk/lib/Basic/Targets/X86.cpp

return llvm::StringSwitch<bool>(Name)
       .Case("sse2", true)
       .Case("sse3", true)
       .Case("ssse3", true)
       .Case("sse4", true)
       .Case("sse4.1", true)
       .Case("sse4.2", true)
       .Case("sse4a", true)
       .Case("tbm", true)
+      .Case("x87", true)
       .Case("xop", true)
       .Case("xsave", true)
       .Case("xsavec", true)
       .Case("xsaves", true)
       .Case("xsaveopt", true)
       .Default(false);
 }
 

cfe/trunk/lib/CodeGen/CGCall.cpp

if (FD && FD->hasAttr<TargetAttr>()) {
         Features.push_back((it->second ? "+" : "-") + it->first().str());
 
       // Now add the target-cpu and target-features to the function.
       // While we populated the feature map above, we still need to
       // get and parse the target attribute so we can get the cpu for
       // the function.
       const auto *TD = FD->getAttr<TargetAttr>();
       TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
-      if (ParsedAttr.Architecture != "")
+      if (ParsedAttr.Architecture != "" &&
+          getTarget().isValidCPUName(ParsedAttr.Architecture))
         TargetCPU = ParsedAttr.Architecture;
       if (TargetCPU != "")
          FuncAttrs.addAttribute("target-cpu", TargetCPU);
       if (!Features.empty()) {
         std::sort(Features.begin(), Features.end());
         FuncAttrs.addAttribute(
             "target-features",
             llvm::join(Features.begin(), Features.end(), ","));
       }
     } else {
       // Otherwise just add the existing target cpu and target features to the

cfe/trunk/lib/CodeGen/CodeGenModule.cpp

 // passed in function.
 void CodeGenModule::getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
                                           const FunctionDecl *FD) {
   StringRef TargetCPU = Target.getTargetOpts().CPU;
   if (const auto *TD = FD->getAttr<TargetAttr>()) {
     // If we have a TargetAttr build up the feature map based on that.
     TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
 
+    ParsedAttr.Features.erase(
+        llvm::remove_if(ParsedAttr.Features,
+                        [&](const std::string &Feat) {
+                          return !Target.isValidFeatureName(
+                              StringRef{Feat}.substr(1));
+                        }),
+        ParsedAttr.Features.end());
+
     // Make a copy of the features as passed on the command line into the
     // beginning of the additional features from the function to override.
     ParsedAttr.Features.insert(ParsedAttr.Features.begin(),
                             Target.getTargetOpts().FeaturesAsWritten.begin(),
                             Target.getTargetOpts().FeaturesAsWritten.end());
 
-    if (ParsedAttr.Architecture != "")
-      TargetCPU = ParsedAttr.Architecture ;
+    if (ParsedAttr.Architecture != "" &&
+        Target.isValidCPUName(ParsedAttr.Architecture))
+      TargetCPU = ParsedAttr.Architecture;
 
     // Now populate the feature map, first with the TargetCPU which is either
     // the default or a new one from the target attribute string. Then we'll use
     // the passed in features (FeaturesAsWritten) along with the new ones from
     // the attribute.
     Target.initFeatureMap(FeatureMap, getDiags(), TargetCPU,
                           ParsedAttr.Features);
   } else {

cfe/trunk/test/CodeGen/attr-target-x86.c

-// RUN: %clang_cc1 -triple x86_64-linux-gnu -target-cpu x86-64 -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple i686-linux-gnu -target-cpu i686 -emit-llvm %s -o - | FileCheck %s
 
 int baz(int a) { return 4; }
 
 int __attribute__((target("avx,sse4.2,arch=ivybridge"))) foo(int a) { return 4; }
 
 int __attribute__((target("tune=sandybridge"))) walrus(int a) { return 4; }
 int __attribute__((target("fpmath=387"))) koala(int a) { return 4; }
 
 int __attribute__((target("no-sse2"))) echidna(int a) { return 4; }
 
 int __attribute__((target("sse4"))) panda(int a) { return 4; }
 int __attribute__((target("no-sse4"))) narwhal(int a) { return 4; }
 
 int bar(int a) { return baz(a) + foo(a); }
 
 int __attribute__((target("avx,      sse4.2,      arch=   ivybridge"))) qux(int a) { return 4; }
 int __attribute__((target("no-aes, arch=ivybridge"))) qax(int a) { return 4; }
 
 int __attribute__((target("no-mmx"))) qq(int a) { return 40; }
 
-int __attribute__((target("arch=lakemont"))) lake(int a) { return 4; }
+int __attribute__((target("arch=lakemont,mmx"))) lake(int a) { return 4; }
 
 // Check that we emit the additional subtarget and cpu features for foo and not for baz or bar.
 // CHECK: baz{{.*}} #0
 // CHECK: foo{{.*}} #1
 // We ignore the tune attribute so walrus should be identical to baz and bar.
 // CHECK: walrus{{.*}} #0
 // We're currently ignoring the fpmath attribute so koala should be identical to baz and bar.
 // CHECK: koala{{.*}} #0
 // CHECK: echidna{{.*}} #2
 // CHECK: panda{{.*}} #3
 // CHECK: narwhal{{.*}} #4
 // CHECK: bar{{.*}} #0
 // CHECK: qux{{.*}} #1
 // CHECK: qax{{.*}} #5
 // CHECK: qq{{.*}} #6
 // CHECK: lake{{.*}} #7
-// CHECK: #0 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87"
+// CHECK: #0 = {{.*}}"target-cpu"="i686" "target-features"="+x87"
 // CHECK: #1 = {{.*}}"target-cpu"="ivybridge" "target-features"="+aes,+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt"
-// CHECK: #2 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+x87,-aes,-avx,-avx2,-avx512bw,-avx512cd,-avx512dq,-avx512er,-avx512f,-avx512ifma,-avx512pf,-avx512vbmi,-avx512vl,-avx512vpopcntdq,-f16c,-fma,-fma4,-pclmul,-sha,-sse2,-sse3,-sse4.1,-sse4.2,-sse4a,-ssse3,-xop,-xsave,-xsaveopt"
-// CHECK: #3 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87"
-// CHECK: #4 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87,-avx,-avx2,-avx512bw,-avx512cd,-avx512dq,-avx512er,-avx512f,-avx512ifma,-avx512pf,-avx512vbmi,-avx512vl,-avx512vpopcntdq,-f16c,-fma,-fma4,-sse4.1,-sse4.2,-xop,-xsave,-xsaveopt"
+// CHECK: #2 = {{.*}}"target-cpu"="i686" "target-features"="+x87,-aes,-avx,-avx2,-avx512bw,-avx512cd,-avx512dq,-avx512er,-avx512f,-avx512ifma,-avx512pf,-avx512vbmi,-avx512vl,-avx512vpopcntdq,-f16c,-fma,-fma4,-pclmul,-sha,-sse2,-sse3,-sse4.1,-sse4.2,-sse4a,-ssse3,-xop,-xsave,-xsaveopt"
+// CHECK: #3 = {{.*}}"target-cpu"="i686" "target-features"="+mmx,+popcnt,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87"
+// CHECK: #4 = {{.*}}"target-cpu"="i686" "target-features"="+x87,-avx,-avx2,-avx512bw,-avx512cd,-avx512dq,-avx512er,-avx512f,-avx512ifma,-avx512pf,-avx512vbmi,-avx512vl,-avx512vpopcntdq,-f16c,-fma,-fma4,-sse4.1,-sse4.2,-xop,-xsave,-xsaveopt"
 // CHECK: #5 = {{.*}}"target-cpu"="ivybridge" "target-features"="+avx,+cx16,+f16c,+fsgsbase,+fxsr,+mmx,+pclmul,+popcnt,+rdrnd,+sse,+sse2,+sse3,+sse4.1,+sse4.2,+ssse3,+x87,+xsave,+xsaveopt,-aes"
-// CHECK: #6 = {{.*}}"target-cpu"="x86-64" "target-features"="+fxsr,+sse,+sse2,+x87,-3dnow,-3dnowa,-mmx"
-// CHECK: #7 = {{.*}}"target-cpu"="lakemont" "target-features"="+mmx,+sse,+sse2"
+// CHECK: #6 = {{.*}}"target-cpu"="i686" "target-features"="+x87,-3dnow,-3dnowa,-mmx"
+// CHECK: #7 = {{.*}}"target-cpu"="lakemont" "target-features"="+mmx"