GBuella updated this revision to Diff 147775.
GBuella added a comment.

Fixed a horrible bug in the patch. Adding a ref to temporary string is not a 
wise thing to do, so I had to remove this line:

  ReqFeatures.push_back(F.substr(1));

Now the ReqFeatures vector can also refer to strings starting with '+'

Also, added a few tests and comments.


https://reviews.llvm.org/D46541

Files:
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGen/target-features-error-2.c
  test/CodeGen/target-features-error.c

Index: test/CodeGen/target-features-error.c
===================================================================
--- test/CodeGen/target-features-error.c
+++ test/CodeGen/target-features-error.c
@@ -3,6 +3,5 @@
   return a + 4;
 }
 int bar() {
-  return foo(4); // expected-error {{always_inline function 'foo' requires target feature 'sse4.2', but would be inlined into function 'bar' that is compiled without support for 'sse4.2'}}
+  return foo(4); // expected-error {{always_inline function 'foo' requires target feature 'avx', but would be inlined into function 'bar' that is compiled without support for 'avx'}}
 }
-
Index: test/CodeGen/target-features-error-2.c
===================================================================
--- test/CodeGen/target-features-error-2.c
+++ test/CodeGen/target-features-error-2.c
@@ -1,38 +1,67 @@
-// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -D NEED_SSE42
 // RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -D NEED_AVX_1
 // RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -D NEED_AVX_2
 // RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -D NEED_AVX_3
 // RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -D NEED_AVX_4
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -D NEED_AVX_5
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -S -verify -o - -D NEED_AVX512f
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -target-feature +movdir64b -S -verify -o - -D NEED_MOVDIRI
+// RUN: %clang_cc1 %s -triple=x86_64-linux-gnu -target-feature +avx512vnni -target-feature +movdiri -S -verify -o - -D NEED_CLWB
 
 #define __MM_MALLOC_H
 #include <x86intrin.h>
 
-#if NEED_SSE42
+#if NEED_AVX_1
 int baz(__m256i a) {
-  return _mm256_extract_epi32(a, 3); // expected-error {{always_inline function '_mm256_extract_epi32' requires target feature 'sse4.2', but would be inlined into function 'baz' that is compiled without support for 'sse4.2'}}
+  return _mm256_extract_epi32(a, 3); // expected-error {{always_inline function '_mm256_extract_epi32' requires target feature 'avx', but would be inlined into function 'baz' that is compiled without support for 'avx'}}
 }
 #endif
 
-#if NEED_AVX_1
+#if NEED_AVX_2
 __m128 need_avx(__m128 a, __m128 b) {
   return _mm_cmp_ps(a, b, 0); // expected-error {{'__builtin_ia32_cmpps' needs target feature avx}}
 }
 #endif
 
-#if NEED_AVX_2
+#if NEED_AVX_3
 __m128 need_avx(__m128 a, __m128 b) {
   return _mm_cmp_ss(a, b, 0); // expected-error {{'__builtin_ia32_cmpss' needs target feature avx}}
 }
 #endif
 
-#if NEED_AVX_3
+#if NEED_AVX_4
 __m128d need_avx(__m128d a, __m128d b) {
   return _mm_cmp_pd(a, b, 0); // expected-error {{'__builtin_ia32_cmppd' needs target feature avx}}
 }
 #endif
 
-#if NEED_AVX_4
+#if NEED_AVX_5
 __m128d need_avx(__m128d a, __m128d b) {
   return _mm_cmp_sd(a, b, 0); // expected-error {{'__builtin_ia32_cmpsd' needs target feature avx}}
 }
 #endif
+
+#if NEED_AVX512f
+unsigned short need_avx512f(unsigned short a, unsigned short b) {
+  return __builtin_ia32_korhi(a, b); // expected-error {{'__builtin_ia32_korhi' needs target feature avx512f}}
+}
+#endif
+
+#if NEED_MOVDIRI
+void need_movdiri(unsigned int *a, unsigned int b) {
+  __builtin_ia32_directstore_u32(a, b); // expected-error {{'__builtin_ia32_directstore_u32' needs target feature movdiri}}
+}
+#endif
+
+#if NEED_CLWB
+static __inline__ void
+ __attribute__((__always_inline__, __nodebug__,  __target__("avx512vnni,clwb,movdiri,movdir64b")))
+ func(unsigned int *a, unsigned int b)
+{
+  __builtin_ia32_directstore_u32(a, b);
+}
+
+void need_clwb(unsigned int *a, unsigned int b) {
+  func(a, b); // expected-error {{always_inline function 'func' requires target feature 'clwb', but would be inlined into function 'need_clwb' that is compiled without support for 'clwb'}}
+
+}
+#endif
Index: lib/CodeGen/CodeGenModule.h
===================================================================
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1089,6 +1089,10 @@
   /// It's up to you to ensure that this is safe.
   void AddDefaultFnAttrs(llvm::Function &F);
 
+  /// Parses the target attributes passed in, and returns only the ones that are
+  /// valid feature names.
+  TargetAttr::ParsedTargetAttr filterFunctionTargetAttrs(const TargetAttr *TD);
+
   // Fills in the supplied string map with the set of target features for the
   // passed in function.
   void getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -5032,22 +5032,28 @@
   }
 }
 
+TargetAttr::ParsedTargetAttr CodeGenModule::filterFunctionTargetAttrs(const TargetAttr *TD) {
+  assert(TD != nullptr);
+  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());
+  return ParsedAttr;
+}
+
+
 // Fills in the supplied string map with the set of target features for the
 // 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());
+    TargetAttr::ParsedTargetAttr ParsedAttr = filterFunctionTargetAttrs(TD);
 
     // Make a copy of the features as passed on the command line into the
     // beginning of the additional features from the function to override.
Index: lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -2287,11 +2287,15 @@
         Feature.split(OrFeatures, '|');
         return std::any_of(OrFeatures.begin(), OrFeatures.end(),
                            [&](StringRef Feature) {
-                             if (!CallerFeatureMap.lookup(Feature)) {
-                               FirstMissing = Feature.str();
-                               return false;
+                             if (Feature[0] == '+') {
+                               if (CallerFeatureMap.lookup(Feature.substr(1)))
+                                 return true;
+                             } else {
+                               if (CallerFeatureMap.lookup(Feature))
+                                 return true;
                              }
-                             return true;
+                             FirstMissing = Feature.str();
+                             return false;
                            });
       });
 }
@@ -2330,17 +2334,30 @@
 
   } else if (TargetDecl->hasAttr<TargetAttr>()) {
     // Get the required features for the callee.
+
+    const TargetAttr *TD = TargetDecl->getAttr<TargetAttr>();
+    TargetAttr::ParsedTargetAttr ParsedAttr = CGM.filterFunctionTargetAttrs(TD);
+
     SmallVector<StringRef, 1> ReqFeatures;
     llvm::StringMap<bool> CalleeFeatureMap;
     CGM.getFunctionFeatureMap(CalleeFeatureMap, TargetDecl);
+
+    for (const auto &F : ParsedAttr.Features) {
+      if (F[0] == '+' && CalleeFeatureMap.lookup(F.substr(1)))
+        ReqFeatures.push_back(F);
+    }
+
     for (const auto &F : CalleeFeatureMap) {
       // Only positive features are "required".
       if (F.getValue())
         ReqFeatures.push_back(F.getKey());
     }
     if (!hasRequiredFeatures(ReqFeatures, CGM, FD, MissingFeature))
       CGM.getDiags().Report(E->getLocStart(), diag::err_function_needs_feature)
-          << FD->getDeclName() << TargetDecl->getDeclName() << MissingFeature;
+          << FD->getDeclName() << TargetDecl->getDeclName()
+          << ((MissingFeature[0] == '+')
+              ? MissingFeature.substr(1)
+	      : MissingFeature);
   }
 }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to