Updates after review comments.

Author: Stylie777

clang/docs/ReleaseNotes.rst

@@ -601,8 +601,8 @@ Arm and AArch64 Support
 - The ``+nosimd`` attribute is now fully supported for ARM. Previously, this had no effect when being used with
   ARM targets, however this will now disable NEON instructions being generated. The ``simd`` option is
   also now printed when the ``--print-supported-extensions`` option is used.
-- NEON is correctly enabled when using features that depend on NEON , and disables all features that depend on
-  NEON when using ``+nosimd``.
+- When a feature that depends on NEON (``simd``) is used, NEON is now automatically enabled.
+- When NEON is disabled (``+nosimd``), all features that depend on NEON will now be disabled.
 
 -  Support for __ptrauth type qualifier has been added.
 

clang/lib/Driver/ToolChains/Arch/ARM.cpp

@@ -781,29 +781,21 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
   if (FPUKind == llvm::ARM::FK_FPV5_D16 || FPUKind == llvm::ARM::FK_FPV5_SP_D16)
     Features.push_back("-mve.fp");
 
-  // If SIMD has been disabled and the selected FPU support NEON, then features
-  // that rely on NEON Instructions should also be disabled. Cases where NEON
-  // needs activating to support another feature is handled below with the
-  // crypto feature.
+  // If SIMD has been disabled and the selected FPU supports NEON, then features
+  // that rely on NEON instructions should also be disabled.
   bool HasSimd = false;
   const auto ItSimd =
       llvm::find_if(llvm::reverse(Features),
                     [](const StringRef F) { return F.contains("neon"); });
-  const bool FoundSimd = ItSimd != Features.rend();
   const bool FPUSupportsNeon = (llvm::ARM::FPUNames[FPUKind].NeonSupport ==
                                 llvm::ARM::NeonSupportLevel::Neon) ||
                                (llvm::ARM::FPUNames[FPUKind].NeonSupport ==
                                 llvm::ARM::NeonSupportLevel::Crypto);
-  if (FoundSimd)
-    HasSimd = ItSimd->take_front() == "+";
-  if (!HasSimd && FPUSupportsNeon) {
-    Features.push_back("-sha2");
-    Features.push_back("-aes");
-    Features.push_back("-crypto");
-    Features.push_back("-dotprod");
-    Features.push_back("-bf16");
-    Features.push_back("-imm8");
-  }
+  if (ItSimd != Features.rend())
+    HasSimd = ItSimd->starts_with("+");
+  if (!HasSimd && FPUSupportsNeon)
+    for (auto &F : {"-sha2", "-aes", "-crypto", "-dotprod", "-bf16", "-imm8"})
+      Features.push_back(F);
 
   // For Arch >= ARMv8.0 && A or R profile:  crypto = sha2 + aes
   // Rather than replace within the feature vector, determine whether each
@@ -840,15 +832,15 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
       llvm::find_if(llvm::reverse(Features),
                     [](const StringRef F) { return F.contains("i8mm"); });
   if (ItSHA2 != Features.rend())
-    HasSHA2 = ItSHA2->take_front() == "+";
+    HasSHA2 = ItSHA2->starts_with("+");
   if (ItAES != Features.rend())
-    HasAES = ItAES->take_front() == "+";
+    HasAES = ItAES->starts_with("+");
   if (ItBF16 != Features.rend())
-    HasBF16 = ItBF16->take_front() == "+";
+    HasBF16 = ItBF16->starts_with("+");
   if (ItDotprod != Features.rend())
-    HasDotprod = ItDotprod->take_front() == "+";
+    HasDotprod = ItDotprod->starts_with("+");
   if (ItI8MM != Features.rend())
-    HasI8MM = ItI8MM->take_front() == "+";
+    HasI8MM = ItI8MM->starts_with("+");
   if (ItCrypto != Features.rend()) {
     if (HasSHA2 && HasAES)
       Features.push_back("+crypto");

clang/test/Driver/arm-features.c

@@ -97,3 +97,11 @@
 // CHECK-NOSHA-DAG: "-target-feature" "-sha2"
 // CHECK-NOAES-DAG: "-target-feature" "-aes"
 //
+// Check that adding features that depend on NEON enable the feature
+// RUN: %clang -target arm-none-none-eabi -march=armv8-r+sha2 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
+// RUN: %clang -target arm-none-none-eabi -march=armv8-r+aes -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
+// RUN: %clang -target arm-none-none-eabi -march=armv8-r+crypto -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
+// RUN: %clang -target arm-none-none-eabi -march=armv8-r+dotprod -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
+// RUN: %clang -target arm-none-none-eabi -march=armv8-r+bf16 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
+// RUN: %clang -target arm-none-none-eabi -march=armv8-r+i8mm -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-NEON-ENABLED-WITH-FEATURE %s
+// CHECK-NEON-ENABLED-WITH-FEATURE: "-target-feature" "+neon"