[AArch64] Allow users-facing feature names in clang target attributes

[dmgreen]

Summary

D133848 added support for the GCC format of target("..") attributes. The supported formats to match gcc are:

//  "arch=<arch>" - parsed to features as per -march=..
//  "cpu=<cpu>" - parsed to features as per -mcpu=.., with CPU set to <cpu>
//  "tune=<cpu>" - TuneCPU set to <cpu>
//  "+feature", "+nofeature" - Add (or remove) feature.

We also support the existing formats, previously accepted by clang, for compatibility with the existing code and intrinsics code:

//  "feature", "no-feature" - Add (or remove) feature.

The clang formats would accept and use internal feature names ("fullfp16"/"neon"/"sve") as opposed to the user facing names ("fp16"/"simd"/"sve"). Usually they use the same names, but can be different for cases like fp, fullfp16 and mte (among others).

This patch makes the clang format also except the user facing names, by parsing the features through getArchExtFeature. There is a fallback if the name is not recognized (like "fullfp16"), where we add the existing string which should then be checked later for consistency. This allows the internal names to be used as before, so long as they are recognized as internal names. (Note that we currently don't have an implementation of isValidFeatureName. The backend will currently give an error like '-sid' is not a recognized feature for this target (ignoring feature). This should be improved in a later patch once an implementation of isValidFeatureName in clang is present).

[danielkiss]

LGTM.

clang/lib/Basic/Targets/AArch64.cpp

auto SplitAndAddFeatures = [](StringRef FeatString,
     FeatString.split(SplitFeatures, StringRef("+"), -1, false);
     for (StringRef Feature : SplitFeatures) {
       StringRef FeatureName = llvm::AArch64::getArchExtFeature(Feature);
       if (!FeatureName.empty())
         Features.push_back(FeatureName.str());
       else
         // Pushing the original feature string to give a sema error later on
         // when they get checked.
-        Features.push_back(Feature.str());
+        if (Feature.startswith("no"))
+          Features.push_back("-" + Feature.drop_front(2).str());
+        else
+          Features.push_back("+" + Feature.str());
     }
   };
 
   for (auto &Feature : AttrFeatures) {
     Feature = Feature.trim();
     if (Feature.startswith("fpmath="))
       continue;
 
if (Feature.startswith("arch=")) {
         Ret.CPU = Split.first;
         SplitAndAddFeatures(Split.second, Ret.Features);
       }
     } else if (Feature.startswith("tune=")) {
       if (!Ret.Tune.empty())
         Ret.Duplicate = "tune=";
       else
         Ret.Tune = Feature.split("=").second.trim();
-    } else if (Feature.startswith("no-"))
-      Ret.Features.push_back("-" + Feature.split("-").second.str());
-    else if (Feature.startswith("+")) {
+    } else if (Feature.startswith("+")) {
       SplitAndAddFeatures(Feature, Ret.Features);
-    }
+    } else if (Feature.startswith("no-")) {
+      StringRef FeatureName =
+          llvm::AArch64::getArchExtFeature(Feature.split("-").second);
+      if (!FeatureName.empty())
+        Ret.Features.push_back("-" + FeatureName.drop_front(1).str());
+      else
+        Ret.Features.push_back("-" + Feature.split("-").second.str());
+    } else {
+      // Try parsing the string to the internal target feature name. If it is
+      // invalid, add the original string (which could already be an internal
+      // name). These should be checked later by isValidFeatureName.
+      StringRef FeatureName = llvm::AArch64::getArchExtFeature(Feature);
+      if (!FeatureName.empty())
+        Ret.Features.push_back(FeatureName.str());
       else
         Ret.Features.push_back("+" + Feature.str());
     }
+  }
   return Ret;
 }
 
 bool AArch64TargetInfo::hasBFloat16Type() const {
   return true;
 }
 
 TargetInfo::CallingConvCheckResult

clang/test/CodeGen/aarch64-targetattr.c

 
 // CHECK-LABEL: @all() #16
 __attribute__((target("cpu=neoverse-n1,tune=cortex-a710,arch=armv8.6-a+sve2")))
 void all() {}
 // CHECK-LABEL: @allplusbranchprotection() #17
 __attribute__((target("cpu=neoverse-n1,tune=cortex-a710,arch=armv8.6-a+sve2,branch-protection=standard")))
 void allplusbranchprotection() {}
 
+// These tests check that the user facing and internal llvm name are both accepted.
+// CHECK-LABEL: @plusnoneon() #18
+__attribute__((target("+noneon")))
+void plusnoneon() {}
+// CHECK-LABEL: @plusnosimd() #18
+__attribute__((target("+nosimd")))
+void plusnosimd() {}
+// CHECK-LABEL: @noneon() #18
+__attribute__((target("no-neon")))
+void noneon() {}
+// CHECK-LABEL: @nosimd() #18
+__attribute__((target("no-simd")))
+void nosimd() {}
+
 // CHECK: attributes #0 = { {{.*}} "target-features"="+v8.1a,+v8.2a,+v8a" }
 // CHECK: attributes #1 = { {{.*}} "target-features"="+sve,+v8.1a,+v8.2a,+v8a" }
 // CHECK: attributes #2 = { {{.*}} "target-features"="+sve2,+v8.1a,+v8.2a,+v8a" }
 // CHECK: attributes #4 = { {{.*}} "target-features"="+sve2,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8.6a,+v8a" }
 // CHECK: attributes #5 = { {{.*}} "target-cpu"="cortex-a710" "target-features"="+bf16,+crc,+dotprod,+flagm,+fp-armv8,+fp16fml,+i8mm,+lse,+mte,+neon,+pauth,+ras,+rcpc,+rdm,+sb,+sve,+sve2,+sve2-bitperm" }
 // CHECK: attributes #6 = { {{.*}} "tune-cpu"="cortex-a710" }
 // CHECK: attributes #7 = { {{.*}} "target-cpu"="generic" }
 // CHECK: attributes #8 = { {{.*}} "tune-cpu"="generic" }
 // CHECK: attributes #9 = { {{.*}} "target-cpu"="neoverse-n1" "target-features"="+crc,+crypto,+dotprod,+fp-armv8,+fullfp16,+lse,+neon,+ras,+rcpc,+rdm,+spe,+ssbs" "tune-cpu"="cortex-a710" }
 // CHECK: attributes #10 = { {{.*}} "target-features"="+sve" "tune-cpu"="cortex-a710" }
 // CHECK: attributes #11 = { {{.*}} "target-cpu"="neoverse-v1" "target-features"="+bf16,+crc,+crypto,+dotprod,+fp-armv8,+fp16fml,+fullfp16,+i8mm,+lse,+neon,+rand,+ras,+rcpc,+rdm,+spe,+ssbs,+sve,+sve2" }
 // CHECK: attributes #12 = { {{.*}} "target-cpu"="neoverse-v1" "target-features"="+bf16,+crc,+crypto,+dotprod,+fp-armv8,+fp16fml,+fullfp16,+i8mm,+lse,+neon,+rand,+ras,+rcpc,+rdm,+spe,+ssbs,-sve" }
 // CHECK: attributes #13 = { {{.*}} "target-features"="+sve" }
 // CHECK: attributes #14 = { {{.*}} "target-features"="+sve,-sve2" }
 // CHECK: attributes #15 = { {{.*}} "target-features"="+fullfp16" }
 // CHECK: attributes #16 = { {{.*}} "target-cpu"="neoverse-n1" "target-features"="+crc,+crypto,+dotprod,+fp-armv8,+fullfp16,+lse,+neon,+ras,+rcpc,+rdm,+spe,+ssbs,+sve2,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8.6a,+v8a" "tune-cpu"="cortex-a710" }
 // CHECK: attributes #17 = { {{.*}} "branch-target-enforcement"="true" {{.*}} "target-cpu"="neoverse-n1" "target-features"="+crc,+crypto,+dotprod,+fp-armv8,+fullfp16,+lse,+neon,+ras,+rcpc,+rdm,+spe,+ssbs,+sve2,+v8.1a,+v8.2a,+v8.3a,+v8.4a,+v8.5a,+v8.6a,+v8a" "tune-cpu"="cortex-a710" }
+// CHECK: attributes #18 = { {{.*}} "target-features"="-neon" }

clang/test/Sema/aarch64-fp16-target.c

 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +neon -fsyntax-only -verify -emit-llvm -o - %s
 // REQUIRES: aarch64-registered-target
 
 // Test that functions with the correct target attributes can use the correct FP16 intrinsics.
 
 #include <arm_fp16.h>
 
 __attribute__((target("fullfp16")))
 void test_fullfp16(float16_t f16) {
   vabdh_f16(f16, f16);
 }
 
+__attribute__((target("fp16")))
+void fp16(float16_t f16) {
+  vabdh_f16(f16, f16);
+}
+
 __attribute__((target("arch=armv8-a+fp16")))
 void test_fp16_arch(float16_t f16) {
     vabdh_f16(f16, f16);
 }
 
 __attribute__((target("+fp16")))
-void test_fp16(float16_t f16) {
+void test_plusfp16(float16_t f16) {
     vabdh_f16(f16, f16);
 }
 
 void undefined(float16_t f16) {
   vabdh_f16(f16, f16); // expected-error {{'__builtin_neon_vabdh_f16' needs target feature fullfp16}}
 }