Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AArch64][TargetParser] autogen ArchExtKind enum #90314

Merged
merged 4 commits into from
Apr 30, 2024

Conversation

tmatheson-arm
Copy link
Contributor

@tmatheson-arm tmatheson-arm commented Apr 27, 2024

Thanks to ExtensionSet::toLLVMFeatureList, all values of ArchExtKind should correspond to a particular -target-feature. The valid values of -target-feature are in turn defined by SubtargetFeature defs.

Therefore we can generate ArchExtKind from the tablegen data. To do this, we consider only SubtargetFeature defs which have a FieldName starting with "Has". This is a bit of magic, which keeps this diff small, but ultimately it will be better to limit this by subclassing SubtargetFeature instead.

Because the Has* FieldNames do not always correspond to the AEK_ names ("extensions", as defined in TargetParser), and AEK_ names do not always correspond to -march strings, some additional enum entries have been added to remap the names. I have renamed these to make the naming consistent, but split them into a separate PR to keep the diff reasonable: #90320

Thanks to ExtensionSet::toLLVMFeatureList, all values of ArchExtKind
should correpond to a particular -target-feature. The valid values of
-target-feature are in turn defined by SubtargetFeature defs.

Therefore we can generate ArchExtKind from the tablegen data. To do
this, we consider only SubtargetFeature defs which have a FieldName
starting with "Has". This is a bit of magic, and it would be better to
limit this by subclassing SubtargetFeature instead.

Because the Has* FieldNames do not always correpond to the AEK_ names,
which do not always correpond to -march strings, some additional enum
entries have been added to remap the names. Follow up commits will
rename either the FieldNames or the AEK_ names or both, in order to make
them all consistent.
This represents the subset of SubtargetFeatures that can be controlled
from the command line (or FMV) and therefore appear in ArchExtKind in
the TargetParser. ArchExtKind is now generated by considering all
intances of Extension in tablegen.

The class enforces consistency between the SubtargetFeature FieldName
("HasXyz") and the ArchExtKind name ("AEK_XYZ").
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 29, 2024

@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-backend-aarch64

Author: Tomas Matheson (tmatheson-arm)

Changes

Thanks to ExtensionSet::toLLVMFeatureList, all values of ArchExtKind should correspond to a particular -target-feature. The valid values of -target-feature are in turn defined by SubtargetFeature defs.

Therefore we can generate ArchExtKind from the tablegen data. To do this, we consider only SubtargetFeature defs which have a FieldName starting with "Has". This is a bit of magic, which keeps this diff small, but ultimately it will be better to limit this by subclassing SubtargetFeature instead.

Because the Has* FieldNames do not always correspond to the AEK_ names ("extensions", as defined in TargetParser), and AEK_ names do not always correspond to -march strings, some additional enum entries have been added to remap the names. I have renamed these to make the naming consistent, but split them into a separate PR to keep the diff reasonable: #90320


Patch is 30.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90314.diff

4 Files Affected:

  • (modified) llvm/include/llvm/TargetParser/AArch64TargetParser.h (+22-81)
  • (modified) llvm/lib/Target/AArch64/AArch64Features.td (+126-115)
  • (modified) llvm/lib/Target/ARM/ARMFeatures.td (+12)
  • (modified) llvm/utils/TableGen/ARMTargetDefEmitter.cpp (+11)
diff --git a/llvm/include/llvm/TargetParser/AArch64TargetParser.h b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
index 0d1cfd152151aa..a0d3d17d99cf74 100644
--- a/llvm/include/llvm/TargetParser/AArch64TargetParser.h
+++ b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
@@ -103,90 +103,31 @@ enum CPUFeatures {
 static_assert(FEAT_MAX < 62,
               "Number of features in CPUFeatures are limited to 62 entries");
 
-// Arch extension modifiers for CPUs. These are labelled with their Arm ARM
-// feature name (though the canonical reference for those is AArch64.td)
-// clang-format off
+// Each ArchExtKind correponds directly to a possible -target-feature.
 enum ArchExtKind : unsigned {
-  AEK_NONE =          1,
-  AEK_CRC =           2,  // FEAT_CRC32
-  AEK_CRYPTO =        3,
-  AEK_FP =            4,  // FEAT_FP
-  AEK_SIMD =          5,  // FEAT_AdvSIMD
-  AEK_FP16 =          6,  // FEAT_FP16
-  AEK_PROFILE =       7,  // FEAT_SPE
-  AEK_RAS =           8,  // FEAT_RAS, FEAT_RASv1p1
-  AEK_LSE =           9,  // FEAT_LSE
-  AEK_SVE =           10, // FEAT_SVE
-  AEK_DOTPROD =       11, // FEAT_DotProd
-  AEK_RCPC =          12, // FEAT_LRCPC
-  AEK_RDM =           13, // FEAT_RDM
-  AEK_SM4 =           14, // FEAT_SM4, FEAT_SM3
-  AEK_SHA3 =          15, // FEAT_SHA3, FEAT_SHA512
-  AEK_SHA2 =          16, // FEAT_SHA1, FEAT_SHA256
-  AEK_AES =           17, // FEAT_AES, FEAT_PMULL
-  AEK_FP16FML =       18, // FEAT_FHM
-  AEK_RAND =          19, // FEAT_RNG
-  AEK_MTE =           20, // FEAT_MTE, FEAT_MTE2
-  AEK_SSBS =          21, // FEAT_SSBS, FEAT_SSBS2
-  AEK_SB =            22, // FEAT_SB
-  AEK_PREDRES =       23, // FEAT_SPECRES
-  AEK_SVE2 =          24, // FEAT_SVE2
-  AEK_SVE2AES =       25, // FEAT_SVE_AES, FEAT_SVE_PMULL128
-  AEK_SVE2SM4 =       26, // FEAT_SVE_SM4
-  AEK_SVE2SHA3 =      27, // FEAT_SVE_SHA3
-  AEK_SVE2BITPERM =   28, // FEAT_SVE_BitPerm
-  AEK_TME =           29, // FEAT_TME
-  AEK_BF16 =          30, // FEAT_BF16
-  AEK_I8MM =          31, // FEAT_I8MM
-  AEK_F32MM =         32, // FEAT_F32MM
-  AEK_F64MM =         33, // FEAT_F64MM
-  AEK_LS64 =          34, // FEAT_LS64, FEAT_LS64_V, FEAT_LS64_ACCDATA
-  AEK_BRBE =          35, // FEAT_BRBE
-  AEK_PAUTH =         36, // FEAT_PAuth
-  AEK_FLAGM =         37, // FEAT_FlagM
-  AEK_SME =           38, // FEAT_SME
-  AEK_SMEF64F64 =     39, // FEAT_SME_F64F64
-  AEK_SMEI16I64 =     40, // FEAT_SME_I16I64
-  AEK_HBC =           41, // FEAT_HBC
-  AEK_MOPS =          42, // FEAT_MOPS
-  AEK_PERFMON =       43, // FEAT_PMUv3
-  AEK_SME2 =          44, // FEAT_SME2
-  AEK_SVE2p1 =        45, // FEAT_SVE2p1
-  AEK_SME2p1 =        46, // FEAT_SME2p1
-  AEK_B16B16 =        47, // FEAT_B16B16
-  AEK_SMEF16F16 =     48, // FEAT_SMEF16F16
-  AEK_CSSC =          49, // FEAT_CSSC
-  AEK_RCPC3 =         50, // FEAT_LRCPC3
-  AEK_THE =           51, // FEAT_THE
-  AEK_D128 =          52, // FEAT_D128
-  AEK_LSE128 =        53, // FEAT_LSE128
-  AEK_SPECRES2 =      54, // FEAT_SPECRES2
-  AEK_RASv2 =         55, // FEAT_RASv2
-  AEK_ITE =           56, // FEAT_ITE
-  AEK_GCS =           57, // FEAT_GCS
-  AEK_FPMR =          58, // FEAT_FPMR
-  AEK_FP8 =           59, // FEAT_FP8
-  AEK_FAMINMAX =      60, // FEAT_FAMINMAX
-  AEK_FP8FMA =        61, // FEAT_FP8FMA
-  AEK_SSVE_FP8FMA =   62, // FEAT_SSVE_FP8FMA
-  AEK_FP8DOT2 =       63, // FEAT_FP8DOT2
-  AEK_SSVE_FP8DOT2 =  64, // FEAT_SSVE_FP8DOT2
-  AEK_FP8DOT4 =       65, // FEAT_FP8DOT4
-  AEK_SSVE_FP8DOT4 =  66, // FEAT_SSVE_FP8DOT4
-  AEK_LUT =           67, // FEAT_LUT
-  AEK_SME_LUTv2 =     68, // FEAT_SME_LUTv2
-  AEK_SMEF8F16 =      69, // FEAT_SME_F8F16
-  AEK_SMEF8F32 =      70, // FEAT_SME_F8F32
-  AEK_SMEFA64 =       71, // FEAT_SME_FA64
-  AEK_CPA =           72, // FEAT_CPA
-  AEK_PAUTHLR =       73, // FEAT_PAuth_LR
-  AEK_TLBIW =         74, // FEAT_TLBIW
-  AEK_JSCVT =         75, // FEAT_JSCVT
-  AEK_FCMA =          76, // FEAT_FCMA
-  AEK_NUM_EXTENSIONS
+  AEK_NONE = 1,
+#define ARM_EXTENSION(NAME, ENUM) ENUM,
+#include "llvm/TargetParser/AArch64TargetParserDef.inc"
+  AEK_NUM_EXTENSIONS,
+
+  // FIXME temporary fixes for inconsistent naming.
+  AEK_F32MM = AEK_MATMULFP32,
+  AEK_F64MM = AEK_MATMULFP64,
+  AEK_FCMA = AEK_COMPLXNUM,
+  AEK_FP = AEK_FPARMV8,
+  AEK_FP16 = AEK_FULLFP16,
+  AEK_I8MM = AEK_MATMULINT8,
+  AEK_JSCVT = AEK_JS,
+  AEK_PROFILE = AEK_SPE,
+  AEK_RASv2 = AEK_RASV2,
+  AEK_RAND = AEK_RANDGEN,
+  AEK_SIMD = AEK_NEON,
+  AEK_SME2p1 = AEK_SME2P1,
+  AEK_SVE2p1 = AEK_SVE2P1,
+  AEK_SME_LUTv2 = AEK_SME_LUTV2,
+
 };
 using ExtensionBitset = Bitset<AEK_NUM_EXTENSIONS>;
-// clang-format on
 
 // Represents an extension that can be enabled with -march=<arch>+<extension>.
 // Typically these correspond to Arm Architecture extensions, unlike
diff --git a/llvm/lib/Target/AArch64/AArch64Features.td b/llvm/lib/Target/AArch64/AArch64Features.td
index 3a3751a85afd1e..a8ad449bd41c86 100644
--- a/llvm/lib/Target/AArch64/AArch64Features.td
+++ b/llvm/lib/Target/AArch64/AArch64Features.td
@@ -9,32 +9,43 @@
 //
 //===----------------------------------------------------------------------===//
 
+// A SubtargetFeature that can be toggled from the command line, and therefore
+// has an AEK_* entry in ArmExtKind.
+class Extension<
+  string TargetFeatureName,            // String used for -target-feature.
+  string Spelling,                     // The XYZ in HasXYZ and AEK_XYZ.
+  string Desc,                         // Description.
+  list<SubtargetFeature> Implies = []  // List of dependent features.
+> : SubtargetFeature<TargetFeatureName, "Has" # Spelling, "true", Desc, Implies>
+{
+    string ArchExtKindSpelling = "AEK_" # Spelling; // ArchExtKind enum name.
+}
+
 // Each SubtargetFeature which corresponds to an Arm Architecture feature should
 // be annotated with the respective FEAT_ feature name from the Architecture
 // Reference Manual. If a SubtargetFeature enables instructions from multiple
 // Arm Architecture Features, it should list all the relevant features. Not all
 // FEAT_ features have a corresponding SubtargetFeature.
 
-def FeatureFPARMv8 : SubtargetFeature<"fp-armv8", "HasFPARMv8", "true",
-                                       "Enable ARMv8 FP (FEAT_FP)">;
+def FeatureFPARMv8 : Extension<"fp-armv8", "FPARMv8", "Enable ARMv8 (FEAT_FP)">;
 
-def FeatureNEON : SubtargetFeature<"neon", "HasNEON", "true",
+def FeatureNEON : Extension<"neon", "NEON",
   "Enable Advanced SIMD instructions (FEAT_AdvSIMD)", [FeatureFPARMv8]>;
 
-def FeatureSM4 : SubtargetFeature<
-    "sm4", "HasSM4", "true",
+def FeatureSM4 : Extension<
+    "sm4", "SM4",
     "Enable SM3 and SM4 support (FEAT_SM4, FEAT_SM3)", [FeatureNEON]>;
 
-def FeatureSHA2 : SubtargetFeature<
-    "sha2", "HasSHA2", "true",
+def FeatureSHA2 : Extension<
+    "sha2", "SHA2",
     "Enable SHA1 and SHA256 support (FEAT_SHA1, FEAT_SHA256)", [FeatureNEON]>;
 
-def FeatureSHA3 : SubtargetFeature<
-    "sha3", "HasSHA3", "true",
+def FeatureSHA3 : Extension<
+    "sha3", "SHA3",
     "Enable SHA512 and SHA3 support (FEAT_SHA3, FEAT_SHA512)", [FeatureNEON, FeatureSHA2]>;
 
-def FeatureAES : SubtargetFeature<
-    "aes", "HasAES", "true",
+def FeatureAES : Extension<
+    "aes", "AES",
     "Enable AES support (FEAT_AES, FEAT_PMULL)", [FeatureNEON]>;
 
 // Crypto has been split up and any combination is now valid (see the
@@ -45,20 +56,20 @@ def FeatureAES : SubtargetFeature<
 // meaning anymore. We kept the Crypto definition here for backward
 // compatibility, and now imply features SHA2 and AES, which was the
 // "traditional" meaning of Crypto.
-def FeatureCrypto : SubtargetFeature<"crypto", "HasCrypto", "true",
+def FeatureCrypto : Extension<"crypto", "Crypto",
   "Enable cryptographic instructions", [FeatureNEON, FeatureSHA2, FeatureAES]>;
 
-def FeatureCRC : SubtargetFeature<"crc", "HasCRC", "true",
+def FeatureCRC : Extension<"crc", "CRC",
   "Enable ARMv8 CRC-32 checksum instructions (FEAT_CRC32)">;
 
-def FeatureRAS : SubtargetFeature<"ras", "HasRAS", "true",
+def FeatureRAS : Extension<"ras", "RAS",
   "Enable ARMv8 Reliability, Availability and Serviceability Extensions (FEAT_RAS, FEAT_RASv1p1)">;
 
-def FeatureRASv2 : SubtargetFeature<"rasv2", "HasRASv2", "true",
+def FeatureRASv2 : Extension<"rasv2", "RASv2",
   "Enable ARMv8.9-A Reliability, Availability and Serviceability Extensions (FEAT_RASv2)",
   [FeatureRAS]>;
 
-def FeatureLSE : SubtargetFeature<"lse", "HasLSE", "true",
+def FeatureLSE : Extension<"lse", "LSE",
   "Enable ARMv8.1 Large System Extension (LSE) atomic instructions (FEAT_LSE)">;
 
 def FeatureLSE2 : SubtargetFeature<"lse2", "HasLSE2", "true",
@@ -70,7 +81,7 @@ def FeatureOutlineAtomics : SubtargetFeature<"outline-atomics", "OutlineAtomics"
 def FeatureFMV : SubtargetFeature<"fmv", "HasFMV", "true",
   "Enable Function Multi Versioning support.">;
 
-def FeatureRDM : SubtargetFeature<"rdm", "HasRDM", "true",
+def FeatureRDM : Extension<"rdm", "RDM",
   "Enable ARMv8.1 Rounding Double Multiply Add/Subtract instructions (FEAT_RDM)",
   [FeatureNEON]>;
 
@@ -91,16 +102,16 @@ def FeatureVH : SubtargetFeature<"vh", "HasVH", "true",
 // This SubtargetFeature is special. It controls only whether codegen will turn
 // `llvm.readcyclecounter()` into an access to a PMUv3 System Register. The
 // `FEAT_PMUv3*` system registers are always available for assembly/disassembly.
-def FeaturePerfMon : SubtargetFeature<"perfmon", "HasPerfMon", "true",
+def FeaturePerfMon : Extension<"perfmon", "PerfMon",
   "Enable Code Generation for ARMv8 PMUv3 Performance Monitors extension (FEAT_PMUv3)">;
 
-def FeatureFullFP16 : SubtargetFeature<"fullfp16", "HasFullFP16", "true",
+def FeatureFullFP16 : Extension<"fullfp16", "FullFP16",
   "Full FP16 (FEAT_FP16)", [FeatureFPARMv8]>;
 
-def FeatureFP16FML : SubtargetFeature<"fp16fml", "HasFP16FML", "true",
+def FeatureFP16FML : Extension<"fp16fml", "FP16FML",
   "Enable FP16 FML instructions (FEAT_FHM)", [FeatureFullFP16]>;
 
-def FeatureSPE : SubtargetFeature<"spe", "HasSPE", "true",
+def FeatureSPE : Extension<"spe", "SPE",
   "Enable Statistical Profiling extension (FEAT_SPE)">;
 
 def FeaturePAN_RWV : SubtargetFeature<
@@ -115,13 +126,13 @@ def FeaturePsUAO : SubtargetFeature< "uaops", "HasPsUAO", "true",
 def FeatureCCPP : SubtargetFeature<"ccpp", "HasCCPP",
     "true", "Enable v8.2 data Cache Clean to Point of Persistence (FEAT_DPB)" >;
 
-def FeatureSVE : SubtargetFeature<"sve", "HasSVE", "true",
+def FeatureSVE : Extension<"sve", "SVE",
   "Enable Scalable Vector Extension (SVE) instructions (FEAT_SVE)", [FeatureFullFP16]>;
 
-def FeatureFPMR : SubtargetFeature<"fpmr", "HasFPMR", "true",
+def FeatureFPMR : Extension<"fpmr", "FPMR",
   "Enable FPMR Register (FEAT_FPMR)">;
 
-def FeatureFP8 : SubtargetFeature<"fp8", "HasFP8", "true",
+def FeatureFP8 : Extension<"fp8", "FP8",
   "Enable FP8 instructions (FEAT_FP8)">;
 
 // This flag is currently still labeled as Experimental, but when fully
@@ -145,33 +156,33 @@ def FeatureExperimentalZeroingPseudos
 def FeatureUseScalarIncVL : SubtargetFeature<"use-scalar-inc-vl",
   "UseScalarIncVL", "true", "Prefer inc/dec over add+cnt">;
 
-def FeatureBF16 : SubtargetFeature<"bf16", "HasBF16",
-    "true", "Enable BFloat16 Extension (FEAT_BF16)" >;
+def FeatureBF16 : Extension<"bf16", "BF16",
+    "Enable BFloat16 Extension (FEAT_BF16)" >;
 
 def FeatureNoSVEFPLD1R : SubtargetFeature<"no-sve-fp-ld1r",
   "NoSVEFPLD1R", "true", "Avoid using LD1RX instructions for FP">;
 
-def FeatureSVE2 : SubtargetFeature<"sve2", "HasSVE2", "true",
+def FeatureSVE2 : Extension<"sve2", "SVE2",
   "Enable Scalable Vector Extension 2 (SVE2) instructions (FEAT_SVE2)",
   [FeatureSVE, FeatureUseScalarIncVL]>;
 
-def FeatureSVE2AES : SubtargetFeature<"sve2-aes", "HasSVE2AES", "true",
+def FeatureSVE2AES : Extension<"sve2-aes", "SVE2AES",
   "Enable AES SVE2 instructions (FEAT_SVE_AES, FEAT_SVE_PMULL128)",
   [FeatureSVE2, FeatureAES]>;
 
-def FeatureSVE2SM4 : SubtargetFeature<"sve2-sm4", "HasSVE2SM4", "true",
+def FeatureSVE2SM4 : Extension<"sve2-sm4", "SVE2SM4",
   "Enable SM4 SVE2 instructions (FEAT_SVE_SM4)", [FeatureSVE2, FeatureSM4]>;
 
-def FeatureSVE2SHA3 : SubtargetFeature<"sve2-sha3", "HasSVE2SHA3", "true",
+def FeatureSVE2SHA3 : Extension<"sve2-sha3", "SVE2SHA3",
   "Enable SHA3 SVE2 instructions (FEAT_SVE_SHA3)", [FeatureSVE2, FeatureSHA3]>;
 
-def FeatureSVE2BitPerm : SubtargetFeature<"sve2-bitperm", "HasSVE2BitPerm", "true",
+def FeatureSVE2BitPerm : Extension<"sve2-bitperm", "SVE2BitPerm",
   "Enable bit permutation SVE2 instructions (FEAT_SVE_BitPerm)", [FeatureSVE2]>;
 
-def FeatureSVE2p1: SubtargetFeature<"sve2p1", "HasSVE2p1", "true",
+def FeatureSVE2p1: Extension<"sve2p1", "SVE2p1",
   "Enable Scalable Vector Extension 2.1 instructions", [FeatureSVE2]>;
 
-def FeatureB16B16 : SubtargetFeature<"b16b16", "HasB16B16", "true",
+def FeatureB16B16 : Extension<"b16b16", "B16B16",
   "Enable SVE2.1 or SME2.1 non-widening BFloat16 to BFloat16 instructions (FEAT_B16B16)", [FeatureBF16]>;
 
 def FeatureZCRegMove : SubtargetFeature<"zcm", "HasZeroCycleRegMove", "true",
@@ -303,23 +314,23 @@ def FeatureForce32BitJumpTables
    : SubtargetFeature<"force-32bit-jump-tables", "Force32BitJumpTables", "true",
                       "Force jump table entries to be 32-bits wide except at MinSize">;
 
-def FeatureRCPC : SubtargetFeature<"rcpc", "HasRCPC", "true",
-                                   "Enable support for RCPC extension (FEAT_LRCPC)">;
+def FeatureRCPC : Extension<"rcpc", "RCPC",
+    "Enable support for RCPC extension (FEAT_LRCPC)">;
 
 def FeatureUseRSqrt : SubtargetFeature<
     "use-reciprocal-square-root", "UseRSqrt", "true",
     "Use the reciprocal square root approximation">;
 
-def FeatureDotProd : SubtargetFeature<
-    "dotprod", "HasDotProd", "true",
+def FeatureDotProd : Extension<
+    "dotprod", "DotProd",
     "Enable dot product support (FEAT_DotProd)", [FeatureNEON]>;
 
-def FeaturePAuth : SubtargetFeature<
-    "pauth", "HasPAuth", "true",
+def FeaturePAuth : Extension<
+    "pauth", "PAuth",
     "Enable v8.3-A Pointer Authentication extension (FEAT_PAuth)">;
 
-def FeatureJS : SubtargetFeature<
-    "jsconv", "HasJS", "true",
+def FeatureJSCVT : Extension<
+    "jsconv", "HasJS",
     "Enable v8.3-A JavaScript FP conversion instructions (FEAT_JSCVT)",
     [FeatureFPARMv8]>;
 
@@ -327,8 +338,8 @@ def FeatureCCIDX : SubtargetFeature<
     "ccidx", "HasCCIDX", "true",
     "Enable v8.3-A Extend of the CCSIDR number of sets (FEAT_CCIDX)">;
 
-def FeatureComplxNum : SubtargetFeature<
-    "complxnum", "HasComplxNum", "true",
+def FeatureFCMA : Extension<
+    "complxnum", "HasComplxNum",
     "Enable v8.3-A Floating-point complex number support (FEAT_FCMA)",
     [FeatureNEON]>;
 
@@ -365,8 +376,8 @@ def FeatureTLB_RMI : SubtargetFeature<
     "tlb-rmi", "HasTLB_RMI", "true",
     "Enable v8.4-A TLB Range and Maintenance Instructions (FEAT_TLBIOS, FEAT_TLBIRANGE)">;
 
-def FeatureFlagM : SubtargetFeature<
-    "flagm", "HasFlagM", "true",
+def FeatureFlagM : Extension<
+    "flagm", "FlagM",
     "Enable v8.4-A Flag Manipulation Instructions (FEAT_FlagM)">;
 
 // 8.4 RCPC enchancements: LDAPR & STLR instructions with Immediate Offset
@@ -414,50 +425,50 @@ def FeatureFRInt3264 : SubtargetFeature<"fptoint", "HasFRInt3264", "true",
 def FeatureSpecRestrict : SubtargetFeature<"specrestrict", "HasSpecRestrict",
   "true", "Enable architectural speculation restriction (FEAT_CSV2_2)">;
 
-def FeatureSB : SubtargetFeature<"sb", "HasSB",
-  "true", "Enable v8.5 Speculation Barrier (FEAT_SB)" >;
+def FeatureSB : Extension<"sb", "SB",
+  "Enable v8.5 Speculation Barrier (FEAT_SB)" >;
 
-def FeatureSSBS : SubtargetFeature<"ssbs", "HasSSBS",
-  "true", "Enable Speculative Store Bypass Safe bit (FEAT_SSBS, FEAT_SSBS2)" >;
+def FeatureSSBS : Extension<"ssbs", "SSBS",
+  "Enable Speculative Store Bypass Safe bit (FEAT_SSBS, FEAT_SSBS2)" >;
 
-def FeaturePredRes : SubtargetFeature<"predres", "HasPredRes", "true",
+def FeaturePredRes : Extension<"predres", "PredRes",
   "Enable v8.5a execution and data prediction invalidation instructions (FEAT_SPECRES)" >;
 
-def FeatureCacheDeepPersist : SubtargetFeature<"ccdp", "HasCCDP",
-    "true", "Enable v8.5 Cache Clean to Point of Deep Persistence (FEAT_DPB2)" >;
+def FeatureCacheDeepPersist : Extension<"ccdp", "CCDP",
+    "Enable v8.5 Cache Clean to Point of Deep Persistence (FEAT_DPB2)" >;
 
-def FeatureBranchTargetId : SubtargetFeature<"bti", "HasBTI",
-    "true", "Enable Branch Target Identification (FEAT_BTI)" >;
+def FeatureBranchTargetId : Extension<"bti", "BTI",
+    "Enable Branch Target Identification (FEAT_BTI)" >;
 
-def FeatureRandGen : SubtargetFeature<"rand", "HasRandGen",
-    "true", "Enable Random Number generation instructions (FEAT_RNG)" >;
+def FeatureRNG : Extension<"rand", "HasRandGen",
+    "Enable Random Number generation instructions (FEAT_RNG)" >;
 
-def FeatureMTE : SubtargetFeature<"mte", "HasMTE",
-    "true", "Enable Memory Tagging Extension (FEAT_MTE, FEAT_MTE2)" >;
+def FeatureMTE : Extension<"mte", "MTE",
+    "Enable Memory Tagging Extension (FEAT_MTE, FEAT_MTE2)" >;
 
-def FeatureTRBE : SubtargetFeature<"trbe", "HasTRBE",
-    "true", "Enable Trace Buffer Extension (FEAT_TRBE)">;
+def FeatureTRBE : Extension<"trbe", "TRBE",
+    "Enable Trace Buffer Extension (FEAT_TRBE)">;
 
-def FeatureETE : SubtargetFeature<"ete", "HasETE",
-    "true", "Enable Embedded Trace Extension (FEAT_ETE)",
+def FeatureETE : Extension<"ete", "ETE",
+    "Enable Embedded Trace Extension (FEAT_ETE)",
     [FeatureTRBE]>;
 
-def FeatureTME : SubtargetFeature<"tme", "HasTME",
-    "true", "Enable Transactional Memory Extension (FEAT_TME)" >;
+def FeatureTME : Extension<"tme", "TME",
+    "Enable Transactional Memory Extension (FEAT_TME)" >;
 
 def FeatureTaggedGlobals : SubtargetFeature<"tagged-globals",
     "AllowTaggedGlobals",
     "true", "Use an instruction sequence for taking the address of a global "
     "that allows a memory tag in the upper address bits">;
 
-def FeatureMatMulInt8 : SubtargetFeature<"i8mm", "HasMatMulInt8",
-    "true", "Enable Matrix Multiply Int8 Extension (FEAT_I8MM)">;
+def FeatureI8MM : Extension<"i8mm", "MatMulInt8",
+    "Enable Matrix Multiply Int8 Extension (FEAT_I8MM)">;
 
-def FeatureMatMulFP32 : SubtargetFeature<"f32mm", "HasMatMulFP32",
-    "true", "Enable Matrix Multiply FP32 Extension (FEAT_F32MM)", [FeatureSVE]>;
+def FeatureF32MM : Extension<"f32mm", "MatMulFP32",
+    "Enable Matrix Multiply FP32 Extension (FEAT_F32MM)", [FeatureSVE]>;
 
-def FeatureMatMulFP64 : SubtargetFeature<"f64mm", "HasMatMulFP64",
-    "true", "Enable Matrix Multiply FP64 Extension (FEAT_F64MM)", [FeatureSVE]>;
+def FeatureF64MM : Extension<"f64mm", "MatMulFP64",
+    "Enable Matrix Multiply FP64 Extension (FEAT_F64MM)", [FeatureSVE]>;
 
 def FeatureXS : SubtargetFeature<"xs", "HasXS",
     "true", "Enable Armv8.7-A limited-TLB-maintenance instruction (FEAT_XS)">;
@@ -468,20 +479,20 @@ def FeatureWFxT : SubtargetFeature<"wfxt", "HasWFxT",
 def FeatureHCX : SubtargetFeature<
     "hcx", "HasHCX", "true", "Enable Armv8.7-A HCRX_EL2 system register (FEAT_HCX)">;
 
-def FeatureLS64 : SubtargetFeature<"ls64", "HasLS64",
-    "true", "Enable Armv8.7-A LD64B/ST64B Accelerator Extension (FEAT_LS64, FEAT_LS64_V, FEAT_LS64_ACCDATA)">;
+def FeatureLS64 : Extension<"ls64", "LS64",
+    "Enable Armv8.7-A LD64B/ST64B Accelerator Extension (FEAT_LS64, FEAT_LS64_V, FEAT_LS64_ACCDATA)">;
 
-def FeatureHBC : SubtargetFeature<"hbc", "HasHBC",
-    "true", "Enable Armv8.8-A Hinted Conditional Branches Extension (FEAT_HBC)">;
+def FeatureHBC : Extension<"hbc", "HBC",
+    "Enable Armv8.8-A Hinted Conditional Branches Extension (FEAT_HBC)">;
 
-def FeatureMOPS : SubtargetFeature<"mops", "HasMOPS",
-    "true", "Enable Armv8.8-A memcpy and memset acceleration instructions (FEAT_MOPS)">;
+def FeatureMOPS : Extension<"mops", "MOPS",
+    "Enable Armv8.8-A memcpy and memset acceleration instructions (FEAT_MOPS)">;
 
 def FeatureNMI : SubtargetFeature<"nmi", "HasNMI",
     "true", "Enable Armv8.8-A Non-maskable Interrupts (FEAT_NMI, FEAT_GICv3_NMI)">;
 
-def FeatureBRBE : SubtargetFeature<"brbe", "HasBRBE",
-    "true", "Enable Branch Record Buffer Extension (FEAT_BRBE)">;
+def FeatureBRBE : Extension<"brbe", "BRBE",
+    "Enable Branch Record Buffer Extension (FEAT_BRBE)">;
 
 def FeatureSPE_EEF : SubtargetFeature<"spe-eef", "HasSPE_EEF",
  ...
[truncated]

@tmatheson-arm
Copy link
Contributor Author

I've added an Extension subclass of SubtargetFeature.

Copy link
Collaborator

@ostannard ostannard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks like a nice clean-up.

@jthackray jthackray self-requested a review April 30, 2024 09:02
Copy link
Contributor

@jthackray jthackray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, a good cleanup of duplicated code, although we will lose the comments where the names don't match, such as AEK_PREDRES // FEAT_SPECRES, AEK_PERFMON // FEAT_PMUv3. I guess we can note them elsewhere.

@tmatheson-arm tmatheson-arm merged commit 61b2a0e into llvm:main Apr 30, 2024
4 checks passed

def FeatureNEON : SubtargetFeature<"neon", "HasNEON", "true",
def FeatureNEON : Extension<"neon", "NEON",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the field name, how are we now handling guarding instructions? There are a number of instances where HasNEON is used to ensure the subtarget has the feature. Will this change affect this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're thinking of the HasNEON Predicate, which is defined in AArch64InstrInfo.td and happens to have the same name. The FieldName being changed here is what defines the name of the field on the AArch64Subtarget C++ class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense - and yes that is the one I am thinking of.

tmatheson-arm added a commit that referenced this pull request Apr 30, 2024
This reverts commit 61b2a0e.

Reason: AArch64TargetParserDef.inc not found while building clang
tmatheson-arm added a commit that referenced this pull request May 1, 2024
Re-land 61b2a0e. Some Windows builds
were failing because AArch64TargetParserDef.inc is a generated header
which is included transitively into some clang components, but this
information is not available to the build system and therefore there is
a missing edge in the dependency graph. This patch incorporates the
fixes described in ac1ffd3/D142403.

Thanks to ExtensionSet::toLLVMFeatureList, all values of ArchExtKind
should correspond to a particular -target-feature. The valid values of
-target-feature are in turn defined by SubtargetFeature defs.

Therefore we can generate ArchExtKind from the tablegen data. This is
done by adding an Extension class which derives from SubtargetFeature.

Because the Has* FieldNames do not always correspond to the AEK_
names ("extensions", as defined in TargetParser), and AEK_ names do
not always correspond to -march strings, some additional enum entries
have been added to remap the names. I have renamed these to make the
naming consistent, but split them into a separate PR to keep the diff
reasonable (#90320)
@tmatheson-arm tmatheson-arm deleted the targetparser_from_tablegen branch May 1, 2024 14:16
tmatheson-arm added a commit that referenced this pull request May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants