[AArch64] Move default extensions from clang Driver to TargetParser

[dmgreen]

Summary

As far as I understand these additions of default extensions would be better added in the TargetParser, not by the driver. This removes the addition of "+i8mm" and "+bf16" features as they are already added in 8.6/9.1 architectures. AEK_MOPS and AEK_HBC have been added to 8.8/9.3 architectures to replace the "+hbc" and "+mops" features.

[lenary]

There's an in-progress version of this for v8.9a in D141404, @pratlucas can you also update that patch maybe?

[tmatheson]

LGTM, makes sense to move into TP, D141404 should do the same it it makes sense.

[pratlucas]

It makes sense to move this to the TargetParser and the code looks much better indeed. My only concern is that this changes the driver's behaviour in regards to which target feature flags are passed from clang to the backend.

It looks like the implied features are included in the backend calls for the -mcpu options and, as far as I can tell, those are correctly captured from the TargetParser info. Should we do the same for -march values? Or should we rely strictly in the backend expansion?

[dmgreen]

In D141518#4047833, @pratlucas wrote:

It makes sense to move this to the TargetParser and the code looks much better indeed. My only concern is that this changes the driver's behaviour in regards to which target feature flags are passed from clang to the backend.

It looks like the implied features are included in the backend calls for the -mcpu options and, as far as I can tell, those are correctly captured from the TargetParser info. Should we do the same for -march values? Or should we rely strictly in the backend expansion?

Hello. Are you looking at a compiler after D141411 went in? I believe after that it should work as you mention. Let me know if it doesn't or you see any problems.

Also I believe that because in the backend HasV8_8aOps includes FeatureMOPS, the feature would already be enabled in codegen (so long as there isn't -target-feature -mops to disable it). Same for other features that become mandatory like dotprod, bf16 and i8mm. We would like them to be "known" by clang too though, so that it can get things like preprocessor defs and especially target("..") attributes correct.

[pratlucas]

Sorry, I just noticed my previous comment wasn't clear on what I meant by passing on the target features.
The features are indeed included as part of the IR attributes since D141411, but they are no longer included as -target-feature arguments in calls to cc1 by the clang driver. This is not the case when using -mcpu, for example.
I agree the end result will be the same assuming the information is consistent between the TargetParser and the backend's subtarget features, but I think it would be good to define the what's the clang driver's expected behaviour to avoid any inconsistencies in the future.

[dmgreen]

In D141518#4047962, @pratlucas wrote:

Sorry, I just noticed my previous comment wasn't clear on what I meant by passing on the target features.
The features are indeed included as part of the IR attributes since D141411, but they are no longer included as -target-feature arguments in calls to cc1 by the clang driver. This is not the case when using -mcpu, for example.
I agree the end result will be the same assuming the information is consistent between the TargetParser and the backend's subtarget features, but I think it would be good to define the what's the clang driver's expected behaviour to avoid any inconsistencies in the future.

Ah I see, between the clang driver and cc1. Yes - I think in the past we were only caring about command line options like -march and -mcpu. In that case there is little difference in practice between adding the logic to the driver and adding it to clang proper. They both have the same effect in the end.
Nowadays we would like for attributes to be able to set architecture features too, as in function-multiversioning and using __attribute__((target("arch=.."))) attributes. When that becomes a consideration it is best for a lot of this logic to be handled in clang-proper, so it can be consistent between the two.

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

if (HasCrypto && !HasNoCrypto) {
         Features.push_back("-aes");
       if (V8Version == 2 || V8Version == 3) {
         Features.push_back("-sm4");
         Features.push_back("-sha3");
       }
     }
   }
 
-  // FIXME: these insertions should ideally be automated using default
-  // extensions support from the backend target parser.
-  if (V8Version >= 6 || V9Version >= 1)
-    Features.insert(std::next(Features.begin() + ArchFeatPos),
-                    {"+i8mm", "+bf16"});
-
-  // For Armv8.8-a/Armv9.3-a or later, FEAT_HBC and FEAT_MOPS are enabled by
-  // default.
-  if (V8Version >= 8 || V9Version >= 3)
-    Features.insert(std::next(Features.begin() + ArchFeatPos),
-                    {"+hbc", "+mops"});
-
   if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
                                options::OPT_munaligned_access)) {
     if (A->getOption().matches(options::OPT_mno_unaligned_access))
       Features.push_back("+strict-align");
   } else if (Triple.isOSOpenBSD())
     Features.push_back("+strict-align");
 
   if (Args.hasArg(options::OPT_ffixed_x1))

clang/test/Driver/aarch64-hbc.c

 // Test that target feature hbc is implemented and available correctly
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.7-a+hbc   %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a       %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+hbc   %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+nohbc %s 2>&1 | FileCheck %s --check-prefix=NO_HBC
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.2-a+hbc   %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a       %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+hbc   %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+nohbc %s 2>&1 | FileCheck %s --check-prefix=NO_HBC
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv8.7-a+hbc   %s 2>&1 | FileCheck %s
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv8.8-a       %s 2>&1 | FileCheck %s
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv8.8-a+hbc   %s 2>&1 | FileCheck %s
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv8.8-a+nohbc %s 2>&1 | FileCheck %s --check-prefix=NO_HBC
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv9.2-a+hbc   %s 2>&1 | FileCheck %s
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv9.3-a       %s 2>&1 | FileCheck %s
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv9.3-a+hbc   %s 2>&1 | FileCheck %s
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv9.3-a+nohbc %s 2>&1 | FileCheck %s --check-prefix=NO_HBC
 
-// CHECK: "-target-feature" "+hbc"
-// NO_HBC: "-target-feature" "-hbc"
+// CHECK: "target-features"="{{.*}},+hbc
+// NO_HBC: "target-features"="{{.*}},-hbc
+
+void test() {}
 No newline at end of file

clang/test/Driver/aarch64-i8mm.c

 // The 8-bit integer matrix multiply extension is a mandatory component of the
 // Armv8.6-A extensions, but is permitted as an optional feature for any
 // implementation of Armv8.2-A to Armv8.5-A (inclusive)
-// RUN: %clang -target aarch64 -march=armv8.5a -### -c %s 2>&1 | FileCheck -check-prefix=NO-I8MM %s
-// RUN: %clang -target aarch64 -march=armv8.5a+i8mm -### -c %s 2>&1 | FileCheck -check-prefix=I8MM %s
-// NO-I8MM-NOT: "-target-feature" "+i8mm"
-// I8MM: "-target-feature" "+i8mm"
+// RUN: %clang -target aarch64 -march=armv8.5a -S -o - -emit-llvm -c %s 2>&1 | FileCheck -check-prefix=NOI8MM %s
+// RUN: %clang -target aarch64 -march=armv8.5a+i8mm -S -o - -emit-llvm -c %s 2>&1 | FileCheck -check-prefix=I8MM %s
+// RUN: %clang -target aarch64 -march=armv8.6a -S -o - -emit-llvm -c %s 2>&1 | FileCheck -check-prefix=I8MM %s
+
+// I8MM: "target-features"="{{.*}},+i8mm
+// NOI8MM-NOT: "target-features"="{{.*}},+i8mm
+
+void test() {}

clang/test/Driver/aarch64-mops.c

 // Test that target feature mops is implemented and available correctly
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.7-a+mops   %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a        %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+mops   %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+nomops %s 2>&1 | FileCheck %s --check-prefix=NO_MOPS
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.2-a+mops   %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a        %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+mops   %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+nomops %s 2>&1 | FileCheck %s --check-prefix=NO_MOPS
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv8.7-a+mops   %s 2>&1 | FileCheck %s
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv8.8-a        %s 2>&1 | FileCheck %s
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv8.8-a+mops   %s 2>&1 | FileCheck %s
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv8.8-a+nomops %s 2>&1 | FileCheck %s --check-prefix=NO_MOPS
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv9.2-a+mops   %s 2>&1 | FileCheck %s
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv9.3-a        %s 2>&1 | FileCheck %s
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv9.3-a+mops   %s 2>&1 | FileCheck %s
+// RUN: %clang -S -o - -emit-llvm -target aarch64-none-none-eabi -march=armv9.3-a+nomops %s 2>&1 | FileCheck %s --check-prefix=NO_MOPS
 
-// CHECK: "-target-feature" "+mops"
-// NO_MOPS: "-target-feature" "-mops"
+// CHECK: "target-features"="{{.*}},+mops
+// NO_MOPS: "target-features"="{{.*}},-mops
+
+void test() {}
 No newline at end of file

clang/test/Driver/aarch64-v91a.c

 // RUN: %clang -target aarch64 -march=armv9.1a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV91A %s
 // RUN: %clang -target aarch64 -march=armv9.1-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV91A %s
 // RUN: %clang -target aarch64 -mlittle-endian -march=armv9.1a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV91A %s
 // RUN: %clang -target aarch64 -mlittle-endian -march=armv9.1-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV91A %s
 // RUN: %clang -target aarch64_be -mlittle-endian -march=armv9.1a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV91A %s
 // RUN: %clang -target aarch64_be -mlittle-endian -march=armv9.1-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV91A %s
-// GENERICV91A: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v9.1a" "-target-feature" "+i8mm" "-target-feature" "+bf16" "-target-feature" "+sve" "-target-feature" "+sve2"
+// GENERICV91A: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v9.1a" "-target-feature" "+sve" "-target-feature" "+sve2"
 
 // RUN: %clang -target aarch64_be -march=armv9.1a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV91A-BE %s
 // RUN: %clang -target aarch64_be -march=armv9.1-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV91A-BE %s
 // RUN: %clang -target aarch64 -mbig-endian -march=armv9.1a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV91A-BE %s
 // RUN: %clang -target aarch64 -mbig-endian -march=armv9.1-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV91A-BE %s
 // RUN: %clang -target aarch64_be -mbig-endian -march=armv9.1a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV91A-BE %s
 // RUN: %clang -target aarch64_be -mbig-endian -march=armv9.1-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV91A-BE %s
-// GENERICV91A-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v9.1a" "-target-feature" "+i8mm" "-target-feature" "+bf16" "-target-feature" "+sve" "-target-feature" "+sve2"
+// GENERICV91A-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v9.1a" "-target-feature" "+sve" "-target-feature" "+sve2"

clang/test/Driver/aarch64-v92a.c

 // RUN: %clang -target aarch64 -march=armv9.2a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV92A %s
 // RUN: %clang -target aarch64 -march=armv9.2-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV92A %s
 // RUN: %clang -target aarch64 -mlittle-endian -march=armv9.2a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV92A %s
 // RUN: %clang -target aarch64 -mlittle-endian -march=armv9.2-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV92A %s
 // RUN: %clang -target aarch64_be -mlittle-endian -march=armv9.2a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV92A %s
 // RUN: %clang -target aarch64_be -mlittle-endian -march=armv9.2-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV92A %s
-// GENERICV92A: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v9.2a" "-target-feature" "+i8mm" "-target-feature" "+bf16" "-target-feature" "+sve" "-target-feature" "+sve2"
+// GENERICV92A: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v9.2a" "-target-feature" "+sve" "-target-feature" "+sve2"
 
 // RUN: %clang -target aarch64_be -march=armv9.2a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV92A-BE %s
 // RUN: %clang -target aarch64_be -march=armv9.2-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV92A-BE %s
 // RUN: %clang -target aarch64 -mbig-endian -march=armv9.2a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV92A-BE %s
 // RUN: %clang -target aarch64 -mbig-endian -march=armv9.2-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV92A-BE %s
 // RUN: %clang -target aarch64_be -mbig-endian -march=armv9.2a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV92A-BE %s
 // RUN: %clang -target aarch64_be -mbig-endian -march=armv9.2-a -### -c %s 2>&1 | FileCheck -check-prefix=GENERICV92A-BE %s
-// GENERICV92A-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v9.2a" "-target-feature" "+i8mm" "-target-feature" "+bf16" "-target-feature" "+sve" "-target-feature" "+sve2"
+// GENERICV92A-BE: "-cc1"{{.*}} "-triple" "aarch64_be{{.*}}" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v9.2a" "-target-feature" "+sve" "-target-feature" "+sve2"

llvm/include/llvm/TargetParser/AArch64TargetParser.h

 inline constexpr ArchInfo ARMV8_1A  = { VersionTuple{8, 1}, AProfile, "armv8.1-a", "+v8.1a", (ARMV8A.DefaultExts | AArch64::AEK_CRC | AArch64::AEK_LSE | AArch64::AEK_RDM)};
 inline constexpr ArchInfo ARMV8_2A  = { VersionTuple{8, 2}, AProfile, "armv8.2-a", "+v8.2a", (ARMV8_1A.DefaultExts | AArch64::AEK_RAS)};
 inline constexpr ArchInfo ARMV8_3A  = { VersionTuple{8, 3}, AProfile, "armv8.3-a", "+v8.3a", (ARMV8_2A.DefaultExts | AArch64::AEK_RCPC)};
 inline constexpr ArchInfo ARMV8_4A  = { VersionTuple{8, 4}, AProfile, "armv8.4-a", "+v8.4a", (ARMV8_3A.DefaultExts | AArch64::AEK_DOTPROD)};
 inline constexpr ArchInfo ARMV8_5A  = { VersionTuple{8, 5}, AProfile, "armv8.5-a", "+v8.5a", (ARMV8_4A.DefaultExts)};
 constexpr unsigned BaseNoCrypto = ARMV8_5A.DefaultExts ^ AArch64::AEK_CRYPTO; // 8.6 onwards has no AEK_CRYPTO
 inline constexpr ArchInfo ARMV8_6A  = { VersionTuple{8, 6}, AProfile, "armv8.6-a", "+v8.6a", (BaseNoCrypto | AArch64::AEK_SM4 | AArch64::AEK_SHA3 | AArch64::AEK_BF16 | AArch64::AEK_SHA2 | AArch64::AEK_AES | AArch64::AEK_I8MM)};
 inline constexpr ArchInfo ARMV8_7A  = { VersionTuple{8, 7}, AProfile, "armv8.7-a", "+v8.7a", (ARMV8_6A.DefaultExts)};
-inline constexpr ArchInfo ARMV8_8A  = { VersionTuple{8, 8}, AProfile, "armv8.8-a", "+v8.8a", (ARMV8_7A.DefaultExts)};
+inline constexpr ArchInfo ARMV8_8A  = { VersionTuple{8, 8}, AProfile, "armv8.8-a", "+v8.8a", (ARMV8_7A.DefaultExts | AArch64::AEK_MOPS | AArch64::AEK_HBC)};
 inline constexpr ArchInfo ARMV8_9A  = { VersionTuple{8, 9}, AProfile, "armv8.9-a", "+v8.9a", (ARMV8_8A.DefaultExts)};
 inline constexpr ArchInfo ARMV9A    = { VersionTuple{9, 0}, AProfile, "armv9-a", "+v9a", (BaseNoCrypto | AArch64::AEK_SVE | AArch64::AEK_SVE2)};
 inline constexpr ArchInfo ARMV9_1A  = { VersionTuple{9, 1}, AProfile, "armv9.1-a", "+v9.1a", (ARMV9A.DefaultExts | AArch64::AEK_BF16 | AArch64::AEK_I8MM)};
 inline constexpr ArchInfo ARMV9_2A  = { VersionTuple{9, 2}, AProfile, "armv9.2-a", "+v9.2a", (ARMV9_1A.DefaultExts)};
-inline constexpr ArchInfo ARMV9_3A  = { VersionTuple{9, 3}, AProfile, "armv9.3-a", "+v9.3a", (ARMV9_2A.DefaultExts)};
+inline constexpr ArchInfo ARMV9_3A  = { VersionTuple{9, 3}, AProfile, "armv9.3-a", "+v9.3a", (ARMV9_2A.DefaultExts | AArch64::AEK_MOPS | AArch64::AEK_HBC)};
 inline constexpr ArchInfo ARMV9_4A  = { VersionTuple{9, 4}, AProfile, "armv9.4-a", "+v9.4a", (ARMV9_3A.DefaultExts)};
 // For v8-R, we do not enable crypto and align with GCC that enables a more minimal set of optional architecture extensions.
 inline constexpr ArchInfo ARMV8R    = { VersionTuple{8, 0}, RProfile, "armv8-r", "+v8r", ((BaseNoCrypto ^ AArch64::AEK_LSE) | AArch64::AEK_SSBS | AArch64::AEK_FP16 | AArch64::AEK_FP16FML | AArch64::AEK_SB), };
 // clang-format on
 
 // The set of all architectures
 static constexpr std::array<const ArchInfo *, 17> ArchInfos = {
     &INVALID,  &ARMV8A,   &ARMV8_1A, &ARMV8_2A, &ARMV8_3A, &ARMV8_4A,

 
 bool isX18ReservedByDefault(const Triple &TT);
 uint64_t getCpuSupportsMask(ArrayRef<StringRef> FeatureStrs);
 
 } // namespace AArch64
 } // namespace llvm
 
 #endif
 No newline at end of file