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]: Refactor target parser to use Bitset. #65423

Merged
merged 8 commits into from
Sep 12, 2023

Conversation

hassnaaHamdi
Copy link
Member

@hassnaaHamdi hassnaaHamdi commented Sep 5, 2023

Use Bitset instead of BitMasking for the Architecture Extensions, because the number of extensions will exceed the bitmask max size.

Use BitVector instead of BitMasking for the Architecture Extensions,
because the number of extensions will exceed the bitmask max size.

Change-Id: Ib4dcf7ee7a400d93ce89c02895ebf4cec2da0f7f
@hassnaaHamdi hassnaaHamdi requested a review from a team as a code owner September 5, 2023 23:40
@github-actions github-actions bot added clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 5, 2023
@DavidSpickett
Copy link
Collaborator

With just the bitvector change this is fine.

I don't disagree with splitting the test files but it should be in its own commit/PR.

Copy link
Collaborator

@sdesmalen-arm sdesmalen-arm left a comment

Choose a reason for hiding this comment

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

Seems like a sensible refactor, we're going to run out of bits eventually.

llvm/include/llvm/TargetParser/AArch64TargetParser.h Outdated Show resolved Hide resolved
llvm/include/llvm/TargetParser/AArch64TargetParser.h Outdated Show resolved Hide resolved
llvm/unittests/TargetParser/AArch64TargetParserTest.cpp Outdated Show resolved Hide resolved
@topperc
Copy link
Collaborator

topperc commented Sep 6, 2023

Does this end up creating a global constructor?

Could llvm::Bitset be used instead. It has a constexpr constructor.

Use Bitset instead of BitMasking for the Architecture Extensions,
because the number of extensions will exceed the bitmask max size.

Change-Id: Id869744514ce89b9ecb50bac033b45eda4d8ceac
@hassnaaHamdi
Copy link
Member Author

Hi,
Thanks for your comments.
I think that I have resolved them.
Now I'm using Bitset instead of Bitvector, as it has constexpr constructor.
Also I dropped splitting the TargetParserTest, and now It's a single test as previous.

@hassnaaHamdi hassnaaHamdi changed the title [AArch64]: Refactor target parser to use BitVector. [AArch64]: Refactor target parser to use Bitset. Sep 7, 2023
Change-Id: I746c95d9cf0006044e89be88447a866462df0515
…f enabled bits.

Fix type.

Change-Id: I23e125c93c82a8fa6fb77b30468ddd5bf7d9c13b
Change-Id: I2494d0d4e59723eba7a206c7615834d522f86a16
llvm/include/llvm/TargetParser/AArch64TargetParser.h Outdated Show resolved Hide resolved
llvm/include/llvm/TargetParser/AArch64TargetParser.h Outdated Show resolved Hide resolved
llvm/unittests/TargetParser/TargetParserTest.cpp Outdated Show resolved Hide resolved
llvm/unittests/TargetParser/TargetParserTest.cpp Outdated Show resolved Hide resolved
llvm/unittests/TargetParser/TargetParserTest.cpp Outdated Show resolved Hide resolved
llvm/unittests/TargetParser/TargetParserTest.cpp Outdated Show resolved Hide resolved
llvm/unittests/TargetParser/TargetParserTest.cpp Outdated Show resolved Hide resolved
for (auto Ext : Extensions)
ExtVal |= Ext;
ExtVal.set(Ext);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does |= not work on Bitset?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's works only for the object. Ext should be an object of Bitset to work.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah of course, that makes sense.

- discard formating that is not related to my changes.
- improve SerializeExtensionFlags() function.
- use ArchExtKind::AEK_EXTENSIONS_LAST instead of AEK_EXTENSIONS_MAX,
  to represent the size of the extensions, given that it's 1-based.

Change-Id: Id3d453ed8cf5a9b3fb170892b8a64793acad4031
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Sep 11, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 11, 2023

@llvm/pr-subscribers-clang

Changes

Use Bitset instead of BitMasking for the Architecture Extensions, because the number of extensions will exceed the bitmask max size.

--

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

5 Files Affected:

  • (modified) clang/lib/Basic/Targets/AArch64.cpp (+1-1)
  • (modified) clang/lib/Driver/ToolChains/Arch/AArch64.cpp (+1-1)
  • (modified) llvm/include/llvm/TargetParser/AArch64TargetParser.h (+281-209)
  • (modified) llvm/lib/TargetParser/AArch64TargetParser.cpp (+4-3)
  • (modified) llvm/unittests/TargetParser/TargetParserTest.cpp (+785-690)
diff --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index 6c43c8b592622d0..78884b7dc1ff6fa 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -968,7 +968,7 @@ bool AArch64TargetInfo::initFeatureMap(
   // Parse the CPU and add any implied features.
   std::optional CpuInfo = llvm::AArch64::parseCpu(CPU);
   if (CpuInfo) {
-    uint64_t Exts = CpuInfo->getImpliedExtensions();
+    llvm::Bitset Exts = CpuInfo->getImpliedExtensions();
     std::vector CPUFeats;
     llvm::AArch64::getExtensionFeatures(Exts, CPUFeats);
     for (auto F : CPUFeats) {
diff --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
index 507ad9247704103..19d454dd8bf3643 100644
--- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -158,7 +158,7 @@ static bool DecodeAArch64Mcpu(const Driver &D, StringRef Mcpu, StringRef &CPU,
 
     Features.push_back(ArchInfo->ArchFeature);
 
-    uint64_t Extension = CpuInfo->getImpliedExtensions();
+    llvm::Bitset Extension = CpuInfo->getImpliedExtensions();
     if (!llvm::AArch64::getExtensionFeatures(Extension, Features))
       return false;
   }
diff --git a/llvm/include/llvm/TargetParser/AArch64TargetParser.h b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
index dc4cdfa8e90ac12..c432718824105e3 100644
--- a/llvm/include/llvm/TargetParser/AArch64TargetParser.h
+++ b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
@@ -15,6 +15,7 @@
 #define LLVM_TARGETPARSER_AARCH64TARGETPARSER_H
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Bitset.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/VersionTuple.h"
 #include 
@@ -96,64 +97,65 @@ static_assert(FEAT_MAX <= 64,
 // 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
-enum ArchExtKind : uint64_t {
-  AEK_NONE =        1,
-  AEK_CRC =         1 << 1,  // FEAT_CRC32
-  AEK_CRYPTO =      1 << 2,
-  AEK_FP =          1 << 3,  // FEAT_FP
-  AEK_SIMD =        1 << 4,  // FEAT_AdvSIMD
-  AEK_FP16 =        1 << 5,  // FEAT_FP16
-  AEK_PROFILE =     1 << 6,  // FEAT_SPE
-  AEK_RAS =         1 << 7,  // FEAT_RAS, FEAT_RASv1p1
-  AEK_LSE =         1 << 8,  // FEAT_LSE
-  AEK_SVE =         1 << 9,  // FEAT_SVE
-  AEK_DOTPROD =     1 << 10, // FEAT_DotProd
-  AEK_RCPC =        1 << 11, // FEAT_LRCPC
-  AEK_RDM =         1 << 12, // FEAT_RDM
-  AEK_SM4 =         1 << 13, // FEAT_SM4, FEAT_SM3
-  AEK_SHA3 =        1 << 14, // FEAT_SHA3, FEAT_SHA512
-  AEK_SHA2 =        1 << 15, // FEAT_SHA1, FEAT_SHA256
-  AEK_AES =         1 << 16, // FEAT_AES, FEAT_PMULL
-  AEK_FP16FML =     1 << 17, // FEAT_FHM
-  AEK_RAND =        1 << 18, // FEAT_RNG
-  AEK_MTE =         1 << 19, // FEAT_MTE, FEAT_MTE2
-  AEK_SSBS =        1 << 20, // FEAT_SSBS, FEAT_SSBS2
-  AEK_SB =          1 << 21, // FEAT_SB
-  AEK_PREDRES =     1 << 22, // FEAT_SPECRES
-  AEK_SVE2 =        1 << 23, // FEAT_SVE2
-  AEK_SVE2AES =     1 << 24, // FEAT_SVE_AES, FEAT_SVE_PMULL128
-  AEK_SVE2SM4 =     1 << 25, // FEAT_SVE_SM4
-  AEK_SVE2SHA3 =    1 << 26, // FEAT_SVE_SHA3
-  AEK_SVE2BITPERM = 1 << 27, // FEAT_SVE_BitPerm
-  AEK_TME =         1 << 28, // FEAT_TME
-  AEK_BF16 =        1 << 29, // FEAT_BF16
-  AEK_I8MM =        1 << 30, // FEAT_I8MM
-  AEK_F32MM =       1ULL << 31, // FEAT_F32MM
-  AEK_F64MM =       1ULL << 32, // FEAT_F64MM
-  AEK_LS64 =        1ULL << 33, // FEAT_LS64, FEAT_LS64_V, FEAT_LS64_ACCDATA
-  AEK_BRBE =        1ULL << 34, // FEAT_BRBE
-  AEK_PAUTH =       1ULL << 35, // FEAT_PAuth
-  AEK_FLAGM =       1ULL << 36, // FEAT_FlagM
-  AEK_SME =         1ULL << 37, // FEAT_SME
-  AEK_SMEF64F64 =   1ULL << 38, // FEAT_SME_F64F64
-  AEK_SMEI16I64 =   1ULL << 39, // FEAT_SME_I16I64
-  AEK_HBC =         1ULL << 40, // FEAT_HBC
-  AEK_MOPS =        1ULL << 41, // FEAT_MOPS
-  AEK_PERFMON =     1ULL << 42, // FEAT_PMUv3
-  AEK_SME2 =        1ULL << 43, // FEAT_SME2
-  AEK_SVE2p1 =      1ULL << 44, // FEAT_SVE2p1
-  AEK_SME2p1 =      1ULL << 45, // FEAT_SME2p1
-  AEK_B16B16 =      1ULL << 46, // FEAT_B16B16
-  AEK_SMEF16F16 =   1ULL << 47, // FEAT_SMEF16F16
-  AEK_CSSC =        1ULL << 48, // FEAT_CSSC
-  AEK_RCPC3 =       1ULL << 49, // FEAT_LRCPC3
-  AEK_THE =         1ULL << 50, // FEAT_THE
-  AEK_D128 =        1ULL << 51, // FEAT_D128
-  AEK_LSE128 =      1ULL << 52, // FEAT_LSE128
-  AEK_SPECRES2 =    1ULL << 53, // FEAT_SPECRES2
-  AEK_RASv2 =       1ULL << 54, // FEAT_RASv2
-  AEK_ITE =         1ULL << 55, // FEAT_ITE
-  AEK_GCS =         1ULL << 56, // FEAT_GCS
+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_EXTENSIONS_LAST =  58
 };
 // clang-format on
 
@@ -273,7 +275,8 @@ struct ArchInfo {
   ArchProfile Profile;   // Architecuture profile
   StringRef Name;        // Human readable name, e.g. "armv8.1-a"
   StringRef ArchFeature; // Command line feature flag, e.g. +v8a
-  uint64_t DefaultExts;  // bitfield of default extensions ArchExtKind
+  Bitset
+      DefaultExts; // bitfield of default extensions ArchExtKind
 
   bool operator==(const ArchInfo &Other) const {
     return this->Name == Other.Name;
@@ -315,23 +318,23 @@ struct ArchInfo {
 };
 
 // clang-format off
-inline constexpr ArchInfo ARMV8A    = { VersionTuple{8, 0}, AProfile, "armv8-a", "+v8a", (AArch64::AEK_FP | AArch64::AEK_SIMD), };
-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 ARMV8A    = { VersionTuple{8, 0}, AProfile, "armv8-a", "+v8a", (Bitset({AArch64::AEK_FP, AArch64::AEK_SIMD})), };
+inline constexpr ArchInfo ARMV8_1A  = { VersionTuple{8, 1}, AProfile, "armv8.1-a", "+v8.1a", (ARMV8A.DefaultExts | Bitset({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 | Bitset({AArch64::AEK_RAS}))};
+inline constexpr ArchInfo ARMV8_3A  = { VersionTuple{8, 3}, AProfile, "armv8.3-a", "+v8.3a", (ARMV8_2A.DefaultExts | Bitset({AArch64::AEK_RCPC}))};
+inline constexpr ArchInfo ARMV8_4A  = { VersionTuple{8, 4}, AProfile, "armv8.4-a", "+v8.4a", (ARMV8_3A.DefaultExts | Bitset({AArch64::AEK_DOTPROD}))};
 inline constexpr ArchInfo ARMV8_5A  = { VersionTuple{8, 5}, AProfile, "armv8.5-a", "+v8.5a", (ARMV8_4A.DefaultExts)};
-inline constexpr ArchInfo ARMV8_6A  = { VersionTuple{8, 6}, AProfile, "armv8.6-a", "+v8.6a", (ARMV8_5A.DefaultExts | AArch64::AEK_BF16 | AArch64::AEK_I8MM)};
+inline constexpr ArchInfo ARMV8_6A  = { VersionTuple{8, 6}, AProfile, "armv8.6-a", "+v8.6a", (ARMV8_5A.DefaultExts | Bitset({AArch64::AEK_BF16, 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 | AArch64::AEK_MOPS | AArch64::AEK_HBC)};
-inline constexpr ArchInfo ARMV8_9A  = { VersionTuple{8, 9}, AProfile, "armv8.9-a", "+v8.9a", (ARMV8_8A.DefaultExts | AArch64::AEK_SPECRES2 | AArch64::AEK_CSSC | AArch64::AEK_RASv2)};
-inline constexpr ArchInfo ARMV9A    = { VersionTuple{9, 0}, AProfile, "armv9-a", "+v9a", (ARMV8_5A.DefaultExts | AArch64::AEK_FP16 | 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 ARMV8_8A  = { VersionTuple{8, 8}, AProfile, "armv8.8-a", "+v8.8a", (ARMV8_7A.DefaultExts | Bitset({AArch64::AEK_MOPS, AArch64::AEK_HBC}))};
+inline constexpr ArchInfo ARMV8_9A  = { VersionTuple{8, 9}, AProfile, "armv8.9-a", "+v8.9a", (ARMV8_8A.DefaultExts | Bitset({AArch64::AEK_SPECRES2, AArch64::AEK_CSSC, AArch64::AEK_RASv2}))};
+inline constexpr ArchInfo ARMV9A    = { VersionTuple{9, 0}, AProfile, "armv9-a", "+v9a", (ARMV8_5A.DefaultExts | Bitset({AArch64::AEK_FP16, AArch64::AEK_SVE, AArch64::AEK_SVE2}))};
+inline constexpr ArchInfo ARMV9_1A  = { VersionTuple{9, 1}, AProfile, "armv9.1-a", "+v9.1a", (ARMV9A.DefaultExts | Bitset({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 | AArch64::AEK_MOPS | AArch64::AEK_HBC)};
-inline constexpr ArchInfo ARMV9_4A  = { VersionTuple{9, 4}, AProfile, "armv9.4-a", "+v9.4a", (ARMV9_3A.DefaultExts | AArch64::AEK_SPECRES2 | AArch64::AEK_CSSC | AArch64::AEK_RASv2)};
+inline constexpr ArchInfo ARMV9_3A  = { VersionTuple{9, 3}, AProfile, "armv9.3-a", "+v9.3a", (ARMV9_2A.DefaultExts | Bitset({AArch64::AEK_MOPS, AArch64::AEK_HBC}))};
+inline constexpr ArchInfo ARMV9_4A  = { VersionTuple{9, 4}, AProfile, "armv9.4-a", "+v9.4a", (ARMV9_3A.DefaultExts | Bitset({AArch64::AEK_SPECRES2, AArch64::AEK_CSSC, AArch64::AEK_RASv2}))};
 // 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", ((ARMV8_5A.DefaultExts ^ AArch64::AEK_LSE) | AArch64::AEK_SSBS | AArch64::AEK_FP16 | AArch64::AEK_FP16FML | AArch64::AEK_SB), };
+inline constexpr ArchInfo ARMV8R    = { VersionTuple{8, 0}, RProfile, "armv8-r", "+v8r", (Bitset(ARMV8_5A.DefaultExts) | Bitset({AArch64::AEK_SSBS, AArch64::AEK_FP16, AArch64::AEK_FP16FML, AArch64::AEK_SB}).flip(AArch64::AEK_LSE))};
 // clang-format on
 
 // The set of all architectures
@@ -345,203 +348,271 @@ static constexpr std::array ArchInfos = {
 struct CpuInfo {
   StringRef Name; // Name, as written for -mcpu.
   const ArchInfo &Arch;
-  uint64_t DefaultExtensions; // Default extensions for this CPU. These will be
-                              // ORd with the architecture defaults.
+  Bitset
+      DefaultExtensions; // Default extensions for this CPU. These will be
+                         // ORd with the architecture defaults.
 
-  uint64_t getImpliedExtensions() const {
-    return DefaultExtensions | Arch.DefaultExts;
+  Bitset getImpliedExtensions() const {
+    Bitset ImpliedExts;
+    ImpliedExts |= DefaultExtensions;
+    ImpliedExts |= Arch.DefaultExts;
+    return ImpliedExts;
   }
 };
 
 inline constexpr CpuInfo CpuInfos[] = {
     {"cortex-a34", ARMV8A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_CRC)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_CRC}))},
     {"cortex-a35", ARMV8A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_CRC)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_CRC}))},
     {"cortex-a53", ARMV8A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_CRC)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_CRC}))},
     {"cortex-a55", ARMV8_2A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_FP16 |
-      AArch64::AEK_DOTPROD | AArch64::AEK_RCPC)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_FP16,
+          AArch64::AEK_DOTPROD, AArch64::AEK_RCPC}))},
     {"cortex-a510", ARMV9A,
-     (AArch64::AEK_BF16 | AArch64::AEK_I8MM | AArch64::AEK_SB |
-      AArch64::AEK_PAUTH | AArch64::AEK_MTE | AArch64::AEK_SSBS |
-      AArch64::AEK_SVE | AArch64::AEK_SVE2 | AArch64::AEK_SVE2BITPERM |
-      AArch64::AEK_FP16FML)},
+     (Bitset(
+         {AArch64::AEK_BF16, AArch64::AEK_I8MM, AArch64::AEK_SB,
+          AArch64::AEK_PAUTH, AArch64::AEK_MTE, AArch64::AEK_SSBS,
+          AArch64::AEK_SVE, AArch64::AEK_SVE2, AArch64::AEK_SVE2BITPERM,
+          AArch64::AEK_FP16FML}))},
     {"cortex-a57", ARMV8A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_CRC)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_CRC}))},
     {"cortex-a65", ARMV8_2A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_DOTPROD |
-      AArch64::AEK_FP16 | AArch64::AEK_RCPC | AArch64::AEK_SSBS)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_DOTPROD,
+          AArch64::AEK_FP16, AArch64::AEK_RCPC, AArch64::AEK_SSBS}))},
     {"cortex-a65ae", ARMV8_2A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_DOTPROD |
-      AArch64::AEK_FP16 | AArch64::AEK_RCPC | AArch64::AEK_SSBS)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_DOTPROD,
+          AArch64::AEK_FP16, AArch64::AEK_RCPC, AArch64::AEK_SSBS}))},
     {"cortex-a72", ARMV8A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_CRC)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_CRC}))},
     {"cortex-a73", ARMV8A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_CRC)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_CRC}))},
     {"cortex-a75", ARMV8_2A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_FP16 |
-      AArch64::AEK_DOTPROD | AArch64::AEK_RCPC)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_FP16,
+          AArch64::AEK_DOTPROD, AArch64::AEK_RCPC}))},
     {"cortex-a76", ARMV8_2A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_FP16 |
-      AArch64::AEK_DOTPROD | AArch64::AEK_RCPC | AArch64::AEK_SSBS)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_FP16,
+          AArch64::AEK_DOTPROD, AArch64::AEK_RCPC, AArch64::AEK_SSBS}))},
     {"cortex-a76ae", ARMV8_2A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_FP16 |
-      AArch64::AEK_DOTPROD | AArch64::AEK_RCPC | AArch64::AEK_SSBS)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_FP16,
+          AArch64::AEK_DOTPROD, AArch64::AEK_RCPC, AArch64::AEK_SSBS}))},
     {"cortex-a77", ARMV8_2A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_FP16 |
-      AArch64::AEK_RCPC | AArch64::AEK_DOTPROD | AArch64::AEK_SSBS)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_FP16,
+          AArch64::AEK_RCPC, AArch64::AEK_DOTPROD, AArch64::AEK_SSBS}))},
     {"cortex-a78", ARMV8_2A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_FP16 |
-      AArch64::AEK_DOTPROD | AArch64::AEK_RCPC | AArch64::AEK_SSBS |
-      AArch64::AEK_PROFILE)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_FP16,
+          AArch64::AEK_DOTPROD, AArch64::AEK_RCPC, AArch64::AEK_SSBS,
+          AArch64::AEK_PROFILE}))},
     {"cortex-a78c", ARMV8_2A,
-     (AArch64::AEK_AES | AArch64::AEK_SHA2 | AArch64::AEK_FP16 |
-      AArch64::AEK_DOTPROD | AArch64::AEK_RCPC | AArch64::AEK_SSBS |
-      AArch64::AEK_PROFILE | AArch64::AEK_FLAGM | AArch64::AEK_PAUTH |
-      AArch64::AEK_FP16FML)},
+     (Bitset(
+         {AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_FP16,
+          AArch64::AEK_DOTPROD, AArch64::AEK_RCPC, AArch64::AEK_SSBS,
+          AArch64::AEK_PROFILE, AArch64::AEK_FLAGM, AArch64::AEK_PAUTH,
+          AArch64::AEK_FP16FML}))},
     {"cortex-a710", ARMV9A,
-     (AArch64::AEK_MTE | AArch64::AEK_PAUTH | AArch64::AEK_FLAGM |
-      AArch64::AEK_SB | AArch64::AEK_I8MM | AArch64::AEK_FP16FML |
-      AArch64::AEK_SVE | AArch64::AEK_SVE2 | AArch64::AEK_SVE2BITPERM |
-      AArch64::AEK_BF16)},
+     (Bitset(
+         {AArch64::AEK_MTE, AArch64::AEK_PAUTH, AArch64::AEK_FLAGM,
+          AArch64::AEK_SB, AArch64::AEK_I8MM, AArch64::AEK_FP16FML,
+          AArch64::AEK_SVE, AArch64::AEK_SVE2, AArch64::AEK_SVE2BITPERM,
+          AArch64::AEK_BF16}))},
     {"cortex-a715", ARMV9A,
-     (AArch64::AEK_SB | AArch64::AEK_SSBS | AArch64::AEK_MTE |
-      AArch64::AEK_FP16 | AArch64::AEK_FP16FML | AArch64::AEK_PAUTH |
-      AArch64::AEK_I8MM | AArch64::AEK_PREDRES | AArch64::AEK_PERFMON |
-      AArch64::AEK_PROFILE | AArch64::AEK_SVE | AArch64::AEK_SVE2BITPERM |
-      AArch64::AEK_BF16 | AArch64::AEK_FLAGM)},
-    {"cortex-r82", ARMV8R...

@hassnaaHamdi
Copy link
Member Author

Hi @sdesmalen-arm
Do you have further comments ?

llvm/include/llvm/TargetParser/AArch64TargetParser.h Outdated Show resolved Hide resolved
llvm/include/llvm/TargetParser/AArch64TargetParser.h Outdated Show resolved Hide resolved
for (auto Ext : Extensions)
ExtVal |= Ext;
ExtVal.set(Ext);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah of course, that makes sense.

llvm/unittests/TargetParser/TargetParserTest.cpp Outdated Show resolved Hide resolved
- Use type alias declaration for Bitset.
- Use function overloading instead of function template,
  to avoid error prone code.

Change-Id: I277c32b10d75e36e3c522ca6f736f23374bc87d6
Copy link
Collaborator

@sdesmalen-arm sdesmalen-arm left a comment

Choose a reason for hiding this comment

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

LGTM

llvm/include/llvm/TargetParser/AArch64TargetParser.h Outdated Show resolved Hide resolved
llvm/include/llvm/TargetParser/AArch64TargetParser.h Outdated Show resolved Hide resolved
- code format.

Change-Id: I5ca736f312a29f86781f29e333f12dcbe9610015
@hassnaaHamdi hassnaaHamdi merged commit 491a1cd into llvm:main Sep 12, 2023
2 checks passed
@hctim
Copy link
Collaborator

hctim commented Sep 12, 2023

Looks like this is causing problems:

https://lab.llvm.org/buildbot/#/builders/18/builds/10698

FAILED: tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/Targets/AArch64.cpp.o 
CCACHE_CPP2=yes CCACHE_HASHDIR=yes /bin/ccache /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm_build0/bin/clang++ -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D_LIBCPP_ENABLE_HARDENED_MODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm_build64/tools/clang/lib/Basic -I/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/clang/lib/Basic -I/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/clang/include -I/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm_build64/tools/clang/include -I/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm_build64/include -I/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/llvm/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -std=c++17 -MD -MT tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/Targets/AArch64.cpp.o -MF tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/Targets/AArch64.cpp.o.d -o tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/Targets/AArch64.cpp.o -c /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/clang/lib/Basic/Targets/AArch64.cpp
In file included from /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/clang/lib/Basic/Targets/AArch64.cpp:13:
In file included from /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/clang/lib/Basic/Targets/AArch64.h:18:
/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/llvm/include/llvm/TargetParser/AArch64TargetParser.h:350:91: error: 'Bitset' may not intend to support class template argument deduction [-Werror,-Wctad-maybe-unsupported]
inline constexpr ArchInfo ARMV8R    = { VersionTuple{8, 0}, RProfile, "armv8-r", "+v8r", (Bitset(ARMV8_5A.DefaultExts) |
                                                                                          ^
/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/llvm/include/llvm/ADT/Bitset.h:31:7: note: add a deduction guide to suppress this warning
class Bitset {
      ^
/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/clang/lib/Basic/Targets/AArch64.cpp:982:5: error: 'Bitset' may not intend to support class template argument deduction [-Werror,-Wctad-maybe-unsupported]
    llvm::Bitset Exts = CpuInfo->getImpliedExtensions();
    ^
/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/llvm/include/llvm/ADT/Bitset.h:31:7: note: add a deduction guide to suppress this warning
class Bitset {
      ^
2 errors generated.

I'm gonna go out on a limb and say this is probably not sanitizer-bot-specific :)

@hassnaaHamdi
Copy link
Member Author

Hi @hctim
Thanks for notifying me :) I see that someone else fixed the issue.

AEK_ITE = 1ULL << 55, // FEAT_ITE
AEK_GCS = 1ULL << 56, // FEAT_GCS
enum ArchExtKind : unsigned {
AEK_NONE = 1,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is bit 0 skipped?

ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
Use Bitset instead of BitMasking for the Architecture Extensions,
as the number of extensions will exceed the bitmask bits eventually.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AArch64 clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants