[mlir] [llvm] [llvm] Remove `inline constexpr` from AArch64TargetParser.h (PR #71601)
Rob Suderman via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 7 15:14:57 PST 2023
https://github.com/rsuderman created https://github.com/llvm/llvm-project/pull/71601
The use of `inline constexpr` appears to be a premature optimization that
can cause failures on some platforms. StringRef has a heavy constructor
that can be missed on dylib loading causing the `Extension` table values
to be incorrect. Switching from `inline constexpr` to `const` results
in no increase in binary size of `llc` indicating the optimization is
unnecessary.
>From 28ac9cbae0eeb0eb026695148e9ba6460ae6c7d2 Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Tue, 31 Oct 2023 10:29:29 -0700
Subject: [PATCH 1/2] [mlir][python] Fix possible use of variable use before
set
The _mlirRegisterEverything symbol may not be built by some customers. The
code here was intended to support this, but didn't properly initialize the
init_module variable.
This would break JAX with:
NameError: free variable 'init_module' referenced before assignment in enclosing scope
---
mlir/python/mlir/_mlir_libs/__init__.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/mlir/python/mlir/_mlir_libs/__init__.py b/mlir/python/mlir/_mlir_libs/__init__.py
index 71c074bc955e8c3..d5fc447e49bf3a6 100644
--- a/mlir/python/mlir/_mlir_libs/__init__.py
+++ b/mlir/python/mlir/_mlir_libs/__init__.py
@@ -83,6 +83,7 @@ def process_initializer_module(module_name):
# If _mlirRegisterEverything is built, then include it as an initializer
# module.
+ init_module = None
if process_initializer_module("_mlirRegisterEverything"):
init_module = importlib.import_module(f"._mlirRegisterEverything", __name__)
>From cd49f372dea864ffc5e015847115b6442ffc0f2d Mon Sep 17 00:00:00 2001
From: Rob Suderman <suderman at google.com>
Date: Tue, 7 Nov 2023 15:11:38 -0800
Subject: [PATCH 2/2] [llvm] Remove `inline constexpr` from
AArch64TargetParser.h
The use of `inline constexpr` appears to be a premature optimization that
can cause failures on some platforms. StringRef has a heavy constructor
that can be missed on dylib loading causing the `Extension` table values
to be incorrect. Switching from `inline constexpr` to `const` results
in no increase in binary size of `llc` indicating the optimization is
unnecessary.
---
.../llvm/TargetParser/AArch64TargetParser.h | 38 +++++++++----------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/llvm/include/llvm/TargetParser/AArch64TargetParser.h b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
index 232b3d6a6dbb1c4..d5b38214b9428aa 100644
--- a/llvm/include/llvm/TargetParser/AArch64TargetParser.h
+++ b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
@@ -189,7 +189,7 @@ struct ExtensionInfo {
// NOTE: If adding a new extension here, consider adding it to ExtensionMap
// in AArch64AsmParser too, if supported as an extension name by binutils.
// clang-format off
-inline constexpr ExtensionInfo Extensions[] = {
+const ExtensionInfo Extensions[] = {
{"aes", AArch64::AEK_AES, "+aes", "-aes", FEAT_AES, "+fp-armv8,+neon", 150},
{"b16b16", AArch64::AEK_B16B16, "+b16b16", "-b16b16", FEAT_INIT, "", 0},
{"bf16", AArch64::AEK_BF16, "+bf16", "-bf16", FEAT_BF16, "+bf16", 280},
@@ -329,35 +329,35 @@ struct ArchInfo {
};
// clang-format off
-inline constexpr ArchInfo ARMV8A = { VersionTuple{8, 0}, AProfile, "armv8-a", "+v8a", (
+const ArchInfo ARMV8A = { VersionTuple{8, 0}, AProfile, "armv8-a", "+v8a", (
AArch64::ExtensionBitset({AArch64::AEK_FP, AArch64::AEK_SIMD})), };
-inline constexpr ArchInfo ARMV8_1A = { VersionTuple{8, 1}, AProfile, "armv8.1-a", "+v8.1a", (ARMV8A.DefaultExts |
+const ArchInfo ARMV8_1A = { VersionTuple{8, 1}, AProfile, "armv8.1-a", "+v8.1a", (ARMV8A.DefaultExts |
AArch64::ExtensionBitset({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 |
+const ArchInfo ARMV8_2A = { VersionTuple{8, 2}, AProfile, "armv8.2-a", "+v8.2a", (ARMV8_1A.DefaultExts |
AArch64::ExtensionBitset({AArch64::AEK_RAS}))};
-inline constexpr ArchInfo ARMV8_3A = { VersionTuple{8, 3}, AProfile, "armv8.3-a", "+v8.3a", (ARMV8_2A.DefaultExts |
+const ArchInfo ARMV8_3A = { VersionTuple{8, 3}, AProfile, "armv8.3-a", "+v8.3a", (ARMV8_2A.DefaultExts |
AArch64::ExtensionBitset({AArch64::AEK_RCPC}))};
-inline constexpr ArchInfo ARMV8_4A = { VersionTuple{8, 4}, AProfile, "armv8.4-a", "+v8.4a", (ARMV8_3A.DefaultExts |
+const ArchInfo ARMV8_4A = { VersionTuple{8, 4}, AProfile, "armv8.4-a", "+v8.4a", (ARMV8_3A.DefaultExts |
AArch64::ExtensionBitset({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 |
+const ArchInfo ARMV8_5A = { VersionTuple{8, 5}, AProfile, "armv8.5-a", "+v8.5a", (ARMV8_4A.DefaultExts)};
+const ArchInfo ARMV8_6A = { VersionTuple{8, 6}, AProfile, "armv8.6-a", "+v8.6a", (ARMV8_5A.DefaultExts |
AArch64::ExtensionBitset({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 |
+const ArchInfo ARMV8_7A = { VersionTuple{8, 7}, AProfile, "armv8.7-a", "+v8.7a", (ARMV8_6A.DefaultExts)};
+const ArchInfo ARMV8_8A = { VersionTuple{8, 8}, AProfile, "armv8.8-a", "+v8.8a", (ARMV8_7A.DefaultExts |
AArch64::ExtensionBitset({AArch64::AEK_MOPS, AArch64::AEK_HBC}))};
-inline constexpr ArchInfo ARMV8_9A = { VersionTuple{8, 9}, AProfile, "armv8.9-a", "+v8.9a", (ARMV8_8A.DefaultExts |
+const ArchInfo ARMV8_9A = { VersionTuple{8, 9}, AProfile, "armv8.9-a", "+v8.9a", (ARMV8_8A.DefaultExts |
AArch64::ExtensionBitset({AArch64::AEK_SPECRES2, AArch64::AEK_CSSC, AArch64::AEK_RASv2}))};
-inline constexpr ArchInfo ARMV9A = { VersionTuple{9, 0}, AProfile, "armv9-a", "+v9a", (ARMV8_5A.DefaultExts |
+const ArchInfo ARMV9A = { VersionTuple{9, 0}, AProfile, "armv9-a", "+v9a", (ARMV8_5A.DefaultExts |
AArch64::ExtensionBitset({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 |
+const ArchInfo ARMV9_1A = { VersionTuple{9, 1}, AProfile, "armv9.1-a", "+v9.1a", (ARMV9A.DefaultExts |
AArch64::ExtensionBitset({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 |
+const ArchInfo ARMV9_2A = { VersionTuple{9, 2}, AProfile, "armv9.2-a", "+v9.2a", (ARMV9_1A.DefaultExts)};
+const ArchInfo ARMV9_3A = { VersionTuple{9, 3}, AProfile, "armv9.3-a", "+v9.3a", (ARMV9_2A.DefaultExts |
AArch64::ExtensionBitset({AArch64::AEK_MOPS, AArch64::AEK_HBC}))};
-inline constexpr ArchInfo ARMV9_4A = { VersionTuple{9, 4}, AProfile, "armv9.4-a", "+v9.4a", (ARMV9_3A.DefaultExts |
+const ArchInfo ARMV9_4A = { VersionTuple{9, 4}, AProfile, "armv9.4-a", "+v9.4a", (ARMV9_3A.DefaultExts |
AArch64::ExtensionBitset({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 |
+const ArchInfo ARMV8R = { VersionTuple{8, 0}, RProfile, "armv8-r", "+v8r", (ARMV8_5A.DefaultExts |
AArch64::ExtensionBitset({AArch64::AEK_SSBS,
AArch64::AEK_FP16, AArch64::AEK_FP16FML, AArch64::AEK_SB}).flip(AArch64::AEK_LSE))};
// clang-format on
@@ -385,7 +385,7 @@ struct CpuInfo {
}
};
-inline constexpr CpuInfo CpuInfos[] = {
+const CpuInfo CpuInfos[] = {
{"cortex-a34", ARMV8A,
(AArch64::ExtensionBitset(
{AArch64::AEK_AES, AArch64::AEK_SHA2, AArch64::AEK_CRC}))},
@@ -646,7 +646,7 @@ struct CpuAlias {
StringRef Name;
};
-inline constexpr CpuAlias CpuAliases[] = {{"grace", "neoverse-v2"}};
+const CpuAlias CpuAliases[] = {{"grace", "neoverse-v2"}};
bool getExtensionFeatures(
const AArch64::ExtensionBitset &Extensions,
More information about the llvm-commits
mailing list