[PATCH] D72762: [ARM][TargetParser] Improve handling of dependencies between target features

[Momchil Velikov via Phabricator via llvm-commits]

chill created this revision.
chill added reviewers: efriedma, SjoerdMeijer, ostannard, labrinea.
Herald added subscribers: llvm-commits, hiraditya, kristof.beyls.
Herald added a project: LLVM.
chill added a child revision: D72633: [ARM][MVE] MVE-I should not be disabled by -mfpu=none.

The patch at https://reviews.llvm.org/D64048 added "negative" dependency
handling in `ARM::appendArchExtFeatures`: feature "noX" removes all features,
which imply "X".

This patch adds the "positive" handling: feature "X" adds all the feature
strings implied by "X".

(This patch also comes from the suggestion here https://reviews.llvm.org/D72633#inline-658582)


https://reviews.llvm.org/D72762

Files:
  clang/test/Preprocessor/arm-target-features.c
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/unittests/Support/TargetParserTest.cpp


Index: llvm/unittests/Support/TargetParserTest.cpp
===================================================================
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -637,6 +637,27 @@
   }
 }
 
+static bool
+testArchExtDependency(const char *ArchExt,
+                      const std::initializer_list<const char *> &Expected) {
+  std::vector<StringRef> Features;
+
+  if (!ARM::appendArchExtFeatures("", ARM::ArchKind::ARMV8_1MMainline, ArchExt,
+                                  Features))
+    return false;
+
+  return llvm::all_of(Expected, [&](StringRef Ext) {
+    return llvm::is_contained(Features, Ext);
+  });
+}
+
+TEST(TargetParserTest, ARMArchExtDependencies) {
+  EXPECT_TRUE(testArchExtDependency("mve", {"+mve", "+dsp"}));
+  EXPECT_TRUE(testArchExtDependency("mve.fp", {"+mve.fp", "+mve", "+dsp"}));
+  EXPECT_TRUE(testArchExtDependency("nodsp", {"-dsp", "-mve", "-mve.fp"}));
+  EXPECT_TRUE(testArchExtDependency("nomve", {"-mve", "-mve.fp"}));
+}
+
 TEST(TargetParserTest, ARMparseHWDiv) {
   const char *hwdiv[] = {"thumb", "arm", "arm,thumb", "thumb,arm"};
 
Index: llvm/lib/Support/ARMTargetParser.cpp
===================================================================
--- llvm/lib/Support/ARMTargetParser.cpp
+++ llvm/lib/Support/ARMTargetParser.cpp
@@ -505,10 +505,13 @@
     return false;
 
   for (const auto AE : ARCHExtNames) {
-    if (Negated && (AE.ID & ID) == ID && AE.NegFeature)
-      Features.push_back(AE.NegFeature);
-    else if (AE.ID == ID && AE.Feature)
-      Features.push_back(AE.Feature);
+    if (Negated) {
+      if ((AE.ID & ID) == ID && AE.NegFeature)
+        Features.push_back(AE.NegFeature);
+    } else {
+      if ((AE.ID & ID) == AE.ID && AE.Feature)
+        Features.push_back(AE.Feature);
+    }
   }
 
   if (CPU == "")
Index: clang/test/Preprocessor/arm-target-features.c
===================================================================
--- clang/test/Preprocessor/arm-target-features.c
+++ clang/test/Preprocessor/arm-target-features.c
@@ -763,7 +763,7 @@
 
 // nofp discards mve.fp
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nofp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVEFP-NOFP %s
-// CHECK-V81M-MVEFP-NOFP-NOT: #define __ARM_FEATURE_MVE
+// CHECK-V81M-MVEFP-NOFP: #define __ARM_FEATURE_MVE 1
 
 // nomve discards mve.fp
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nomve -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVEFP-NOMVE %s
@@ -773,11 +773,16 @@
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve+fp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-FP %s
 // CHECK-V81M-MVE-FP: #define __ARM_FEATURE_MVE 1
 
-// nodsp discards both dsp and mve
+// nodsp discards both dsp and mve ...
 // RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve+nodsp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-NODSP %s
 // CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_MVE
 // CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_DSP
 
+// ... and also mve.fp
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nodsp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-NODSP %s
+// CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_MVE
+// CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_DSP
+
 // RUN: %clang -target armv8.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81A %s
 // CHECK-V81A: #define __ARM_ARCH 8
 // CHECK-V81A: #define __ARM_ARCH_8_1A__ 1


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72762.238229.patch
Type: text/x-patch
Size: 3677 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200115/eb114dff/attachment.bin>