[ARM][TargetParser] Improve handling of dependencies between target features

[chill]

Summary

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)

[efriedma]

LGTM

clang/lib/Basic/Targets/ARM.cpp

if (Feature == "+soft-float") {
       Unaligned = 0;
     } else if (Feature == "+fp16") {
       HW_FP |= HW_FP_HP;
     } else if (Feature == "+fullfp16") {
       HasLegalHalfType = true;
     } else if (Feature == "+dotprod") {
       DotProd = true;
     } else if (Feature == "+mve") {
-      DSP = 1;
       MVE |= MVE_INT;
     } else if (Feature == "+mve.fp") {
-      DSP = 1;
       HasLegalHalfType = true;
       FPU |= FPARMV8;
       MVE |= MVE_INT | MVE_FP;
       HW_FP |= HW_FP_SP | HW_FP_HP;
     }
   }
 
   switch (ArchVersion) {

clang/test/Driver/arm-mfpu.c

 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-vfp2sp"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-vfp3d16sp"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-vfp4d16sp"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-fp-armv8d16sp"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-fp64"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-d32"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-neon"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-crypto"
+// CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "+mve"
+// CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "+dsp"
 // CHECK-MVEFP-FPUNONE-DAG: "-target-feature" "-mve.fp"
 // CHECK-MVEFP-FPUNONE-NOT: "-target-feature" "-fpregs"
 
-
 // RUN: %clang -target arm-none-none-eabi %s -march=armv8.1-m.main+mve -mfpu=none -### -c 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-MVEI-FPUNONE %s
 // CHECK-MVEI-FPUNONE-DAG: "-target-feature" "-mve.fp"
 // CHECK-MVEI-FPUNONE-DAG: "-target-feature" "+mve"
+// CHECK-MVEI-FPUNONE-DAG: "-target-feature" "+dsp"
 // CHECK-MVEI-FPUNONE-NOT: "-target-feature" "-fpregs"

clang/test/Preprocessor/arm-target-features.c

 
 // 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-MVEFP %s
 // CHECK-V81M-MVEFP: #define __ARM_FEATURE_DSP 1
 // CHECK-V81M-MVEFP: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
 // CHECK-V81M-MVEFP: #define __ARM_FEATURE_MVE 3
 // CHECK-V81M-MVEFP: #define __ARM_FEATURE_SIMD32 1
 // CHECK-V81M-MVEFP: #define __ARM_FPV5__ 1
 
-// nofp discards mve.fp
+// nofp discards mve.fp, but not mve/dsp
 // 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_DSP 1
+// 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
 // CHECK-V81M-MVEFP-NOMVE-NOT: #define __ARM_FEATURE_MVE
 
 // mve+fp doesn't imply mve.fp
 // 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
 // CHECK-V81A: #define __ARM_ARCH_PROFILE 'A'
 // CHECK-V81A: #define __ARM_FEATURE_QRDMX 1
 // CHECK-V81A: #define __ARM_FP 0xe
 
 // RUN: %clang -target armv8.2a-none-none-eabi -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V82A %s

llvm/lib/Support/ARMTargetParser.cpp

bool ARM::appendArchExtFeatures(
   size_t StartingNumFeatures = Features.size();
   const bool Negated = stripNegationPrefix(ArchExt);
   uint64_t ID = parseArchExt(ArchExt);
 
   if (ID == AEK_INVALID)
     return false;
 
   for (const auto AE : ARCHExtNames) {
-    if (Negated && (AE.ID & ID) == ID && AE.NegFeature)
+    if (Negated) {
+      if ((AE.ID & ID) == ID && AE.NegFeature)
         Features.push_back(AE.NegFeature);
-    else if (AE.ID == ID && AE.Feature)
+    } else {
+      if ((AE.ID & ID) == AE.ID && AE.Feature)
         Features.push_back(AE.Feature);
     }
+  }
 
   if (CPU == "")
     CPU = "generic";
 
   if (ArchExt == "fp" || ArchExt == "fp.dp") {
     unsigned FPUKind;
     if (ArchExt == "fp.dp") {
       if (Negated) {

llvm/unittests/Support/TargetParserTest.cpp

const char *ArchExt[][4] = {{"crc", "nocrc", "+crc", "-crc"},
                               {"mve.fp", "nomve.fp", "+mve.fp", "-mve.fp"}};
 
   for (unsigned i = 0; i < array_lengthof(ArchExt); i++) {
     EXPECT_EQ(StringRef(ArchExt[i][2]), ARM::getArchExtFeature(ArchExt[i][0]));
     EXPECT_EQ(StringRef(ArchExt[i][3]), ARM::getArchExtFeature(ArchExt[i][1]));
   }
 }
 
+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"};
 
   for (unsigned i = 0; i < array_lengthof(hwdiv); i++)
     EXPECT_NE(ARM::AEK_INVALID, ARM::parseHWDiv((StringRef)hwdiv[i]));
 }
 
 TEST(TargetParserTest, ARMparseArchEndianAndISA) {