[Lldb-commits] [clang] [lldb] [llvm] [llvm][TargetParser] Return optional from getHostCPUFeatures (PR #97824)
David Spickett via lldb-commits
lldb-commits at lists.llvm.org
Fri Jul 5 08:08:29 PDT 2024
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/97824
>From 7ebe4e487b763ff26fbab6d75aa7c8694d63e8b1 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Fri, 5 Jul 2024 08:42:22 +0000
Subject: [PATCH 1/3] [llvm][TargetParser] Return optional from
getHostCPUFeatures
Previously this took a reference to a map and returned a bool
to say whether it succeeded. This is an optional but with more
steps.
The only reason to keep it that way was if someone was appending
to an existing map, but all callers made a new map before calling
it.
---
clang/lib/Driver/ToolChains/Arch/ARM.cpp | 6 ++--
clang/lib/Driver/ToolChains/Arch/X86.cpp | 6 ++--
lldb/utils/lit-cpuid/lit-cpuid.cpp | 21 +++++++-------
llvm/include/llvm/TargetParser/Host.h | 14 ++++-----
llvm/lib/CodeGen/CommandFlags.cpp | 16 ++++------
.../Orc/JITTargetMachineBuilder.cpp | 8 ++---
llvm/lib/Target/TargetMachineC.cpp | 5 ++--
llvm/lib/TargetParser/Host.cpp | 29 ++++++++++++-------
8 files changed, 54 insertions(+), 51 deletions(-)
diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 8ae22cc37a1368..77adbf3865ab11 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -591,9 +591,9 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver &D,
// Add CPU features for generic CPUs
if (CPUName == "native") {
- llvm::StringMap<bool> HostFeatures;
- if (llvm::sys::getHostCPUFeatures(HostFeatures))
- for (auto &F : HostFeatures)
+ if (std::optional<llvm::StringMap<bool>> HostFeatures =
+ llvm::sys::getHostCPUFeatures())
+ for (auto &F : *HostFeatures)
Features.push_back(
Args.MakeArgString((F.second ? "+" : "-") + F.first()));
} else if (!CPUName.empty()) {
diff --git a/clang/lib/Driver/ToolChains/Arch/X86.cpp b/clang/lib/Driver/ToolChains/Arch/X86.cpp
index 92821b2a82daec..e4adfcac23ca0d 100644
--- a/clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -131,9 +131,9 @@ void x86::getX86TargetFeatures(const Driver &D, const llvm::Triple &Triple,
// If -march=native, autodetect the feature list.
if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
if (StringRef(A->getValue()) == "native") {
- llvm::StringMap<bool> HostFeatures;
- if (llvm::sys::getHostCPUFeatures(HostFeatures))
- for (auto &F : HostFeatures)
+ if (std::optional<llvm::StringMap<bool>> HostFeatures =
+ llvm::sys::getHostCPUFeatures())
+ for (auto &F : *HostFeatures)
Features.push_back(
Args.MakeArgString((F.second ? "+" : "-") + F.first()));
}
diff --git a/lldb/utils/lit-cpuid/lit-cpuid.cpp b/lldb/utils/lit-cpuid/lit-cpuid.cpp
index be322cb6aa42a9..16743164e6a5d9 100644
--- a/lldb/utils/lit-cpuid/lit-cpuid.cpp
+++ b/lldb/utils/lit-cpuid/lit-cpuid.cpp
@@ -15,22 +15,23 @@
#include "llvm/Support/raw_ostream.h"
#include "llvm/TargetParser/Host.h"
+#include <optional>
+
using namespace llvm;
int main(int argc, char **argv) {
#if defined(__i386__) || defined(_M_IX86) || \
defined(__x86_64__) || defined(_M_X64)
- StringMap<bool> features;
-
- if (!sys::getHostCPUFeatures(features))
+ if (std::optional<StringMap<bool>> features =
+ sys::getHostCPUFeatures(features)) {
+ if ((*features)["sse"])
+ outs() << "sse\n";
+ if ((*features)["avx"])
+ outs() << "avx\n";
+ if ((*features)["avx512f"])
+ outs() << "avx512f\n";
+ } else
return 1;
-
- if (features["sse"])
- outs() << "sse\n";
- if (features["avx"])
- outs() << "avx\n";
- if (features["avx512f"])
- outs() << "avx512f\n";
#endif
return 0;
diff --git a/llvm/include/llvm/TargetParser/Host.h b/llvm/include/llvm/TargetParser/Host.h
index af72045a8fe67f..d68655835a3233 100644
--- a/llvm/include/llvm/TargetParser/Host.h
+++ b/llvm/include/llvm/TargetParser/Host.h
@@ -13,6 +13,7 @@
#ifndef LLVM_TARGETPARSER_HOST_H
#define LLVM_TARGETPARSER_HOST_H
+#include <optional>
#include <string>
namespace llvm {
@@ -47,13 +48,12 @@ namespace sys {
/// The particular format of the names are target dependent, and suitable for
/// passing as -mattr to the target which matches the host.
///
- /// \param Features - A string mapping feature names to either
- /// true (if enabled) or false (if disabled). This routine makes no guarantees
- /// about exactly which features may appear in this map, except that they are
- /// all valid LLVM feature names.
- ///
- /// \return - True on success.
- bool getHostCPUFeatures(StringMap<bool, MallocAllocator> &Features);
+ /// \return - If feature detection succeeds, a string map mapping feature
+ /// names to either true (if enabled) or false (if disabled). This routine
+ /// makes no guarantees about exactly which features may appear in this map,
+ /// except that they are all valid LLVM feature names. If feature detection
+ /// fails, an empty optional is returned.
+ std::optional<StringMap<bool, MallocAllocator>> getHostCPUFeatures();
/// This is a function compatible with cl::AddExtraVersionPrinter, which adds
/// info about the current target triple and detected CPU.
diff --git a/llvm/lib/CodeGen/CommandFlags.cpp b/llvm/lib/CodeGen/CommandFlags.cpp
index 8fc65d78ff2c90..b881f1289e9f39 100644
--- a/llvm/lib/CodeGen/CommandFlags.cpp
+++ b/llvm/lib/CodeGen/CommandFlags.cpp
@@ -624,12 +624,10 @@ std::string codegen::getFeaturesStr() {
// This is necessary for x86 where the CPU might not support all the
// features the autodetected CPU name lists in the target. For example,
// not all Sandybridge processors support AVX.
- if (getMCPU() == "native") {
- StringMap<bool> HostFeatures;
- if (sys::getHostCPUFeatures(HostFeatures))
- for (const auto &[Feature, IsEnabled] : HostFeatures)
+ if (getMCPU() == "native")
+ if (std::optional<StringMap<bool>> HostFeatures = sys::getHostCPUFeatures())
+ for (const auto &[Feature, IsEnabled] : *HostFeatures)
Features.AddFeature(Feature, IsEnabled);
- }
for (auto const &MAttr : getMAttrs())
Features.AddFeature(MAttr);
@@ -644,12 +642,10 @@ std::vector<std::string> codegen::getFeatureList() {
// This is necessary for x86 where the CPU might not support all the
// features the autodetected CPU name lists in the target. For example,
// not all Sandybridge processors support AVX.
- if (getMCPU() == "native") {
- StringMap<bool> HostFeatures;
- if (sys::getHostCPUFeatures(HostFeatures))
- for (const auto &[Feature, IsEnabled] : HostFeatures)
+ if (getMCPU() == "native")
+ if (std::optional<StringMap<bool>> HostFeatures = sys::getHostCPUFeatures())
+ for (const auto &[Feature, IsEnabled] : *HostFeatures)
Features.AddFeature(Feature, IsEnabled);
- }
for (auto const &MAttr : getMAttrs())
Features.AddFeature(MAttr);
diff --git a/llvm/lib/ExecutionEngine/Orc/JITTargetMachineBuilder.cpp b/llvm/lib/ExecutionEngine/Orc/JITTargetMachineBuilder.cpp
index 540728ea0fd999..5f5a8f71689989 100644
--- a/llvm/lib/ExecutionEngine/Orc/JITTargetMachineBuilder.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/JITTargetMachineBuilder.cpp
@@ -28,10 +28,10 @@ Expected<JITTargetMachineBuilder> JITTargetMachineBuilder::detectHost() {
// Retrieve host CPU name and sub-target features and add them to builder.
// Relocation model, code model and codegen opt level are kept to default
// values.
- llvm::StringMap<bool> FeatureMap;
- llvm::sys::getHostCPUFeatures(FeatureMap);
- for (auto &Feature : FeatureMap)
- TMBuilder.getFeatures().AddFeature(Feature.first(), Feature.second);
+ if (std::optional<StringMap<bool>> FeatureMap =
+ llvm::sys::getHostCPUFeatures())
+ for (auto &Feature : *FeatureMap)
+ TMBuilder.getFeatures().AddFeature(Feature.first(), Feature.second);
TMBuilder.setCPU(std::string(llvm::sys::getHostCPUName()));
diff --git a/llvm/lib/Target/TargetMachineC.cpp b/llvm/lib/Target/TargetMachineC.cpp
index 80024f9a6d5df5..340ae502859513 100644
--- a/llvm/lib/Target/TargetMachineC.cpp
+++ b/llvm/lib/Target/TargetMachineC.cpp
@@ -363,10 +363,9 @@ char *LLVMGetHostCPUName(void) {
char *LLVMGetHostCPUFeatures(void) {
SubtargetFeatures Features;
- StringMap<bool> HostFeatures;
- if (sys::getHostCPUFeatures(HostFeatures))
- for (const auto &[Feature, IsEnabled] : HostFeatures)
+ if (std::optional<StringMap<bool>> HostFeatures = sys::getHostCPUFeatures())
+ for (const auto &[Feature, IsEnabled] : *HostFeatures)
Features.AddFeature(Feature, IsEnabled);
return strdup(Features.getString().c_str());
diff --git a/llvm/lib/TargetParser/Host.cpp b/llvm/lib/TargetParser/Host.cpp
index 2ea56746aff249..4cf06505e902e4 100644
--- a/llvm/lib/TargetParser/Host.cpp
+++ b/llvm/lib/TargetParser/Host.cpp
@@ -1710,15 +1710,17 @@ VendorSignatures getVendorSignature(unsigned *MaxLeaf) {
#if defined(__i386__) || defined(_M_IX86) || \
defined(__x86_64__) || defined(_M_X64)
-bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
+std::optional<StringMap<bool>> sys::getHostCPUFeatures() {
unsigned EAX = 0, EBX = 0, ECX = 0, EDX = 0;
unsigned MaxLevel;
if (getX86CpuIDAndInfo(0, &MaxLevel, &EBX, &ECX, &EDX) || MaxLevel < 1)
- return false;
+ return {};
getX86CpuIDAndInfo(1, &EAX, &EBX, &ECX, &EDX);
+ StringMap<bool> Features;
+
Features["cx8"] = (EDX >> 8) & 1;
Features["cmov"] = (EDX >> 15) & 1;
Features["mmx"] = (EDX >> 23) & 1;
@@ -1903,13 +1905,13 @@ bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
Features["avx10.1-512"] =
Features["avx10.1-256"] && HasLeaf24 && ((EBX >> 18) & 1);
- return true;
+ return Features;
}
#elif defined(__linux__) && (defined(__arm__) || defined(__aarch64__))
-bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
+std::optional<StringMap<bool>> sys::getHostCPUFeatures() {
std::unique_ptr<llvm::MemoryBuffer> P = getProcCpuinfoContent();
if (!P)
- return false;
+ return {};
SmallVector<StringRef, 32> Lines;
P->getBuffer().split(Lines, "\n");
@@ -1929,6 +1931,7 @@ bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
uint32_t crypto = 0;
#endif
+ StringMap<bool> Features;
for (unsigned I = 0, E = CPUFeatures.size(); I != E; ++I) {
StringRef LLVMFeatureStr = StringSwitch<StringRef>(CPUFeatures[I])
#if defined(__aarch64__)
@@ -1972,10 +1975,12 @@ bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
Features["crypto"] = true;
#endif
- return true;
+ return Features;
}
#elif defined(_WIN32) && (defined(__aarch64__) || defined(_M_ARM64))
-bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
+std::optional<StringMap<bool>> sys::getHostCPUFeatures() {
+ std::optional<StringMap<bool>> Features;
+
if (IsProcessorFeaturePresent(PF_ARM_NEON_INSTRUCTIONS_AVAILABLE))
Features["neon"] = true;
if (IsProcessorFeaturePresent(PF_ARM_V8_CRC32_INSTRUCTIONS_AVAILABLE))
@@ -1983,16 +1988,18 @@ bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
if (IsProcessorFeaturePresent(PF_ARM_V8_CRYPTO_INSTRUCTIONS_AVAILABLE))
Features["crypto"] = true;
- return true;
+ return Features;
}
#elif defined(__linux__) && defined(__loongarch__)
#include <sys/auxv.h>
-bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
+std::optional<StringMap<bool>> sys::getHostCPUFeatures() {
unsigned long hwcap = getauxval(AT_HWCAP);
bool HasFPU = hwcap & (1UL << 3); // HWCAP_LOONGARCH_FPU
uint32_t cpucfg2 = 0x2;
__asm__("cpucfg %[cpucfg2], %[cpucfg2]\n\t" : [cpucfg2] "+r"(cpucfg2));
+ std::optional<StringMap<bool>> Features;
+
Features["f"] = HasFPU && (cpucfg2 & (1U << 1)); // CPUCFG.2.FP_SP
Features["d"] = HasFPU && (cpucfg2 & (1U << 2)); // CPUCFG.2.FP_DP
@@ -2000,10 +2007,10 @@ bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
Features["lasx"] = hwcap & (1UL << 5); // HWCAP_LOONGARCH_LASX
Features["lvz"] = hwcap & (1UL << 9); // HWCAP_LOONGARCH_LVZ
- return true;
+ return Features;
}
#else
-bool sys::getHostCPUFeatures(StringMap<bool> &Features) { return false; }
+std::optional<StringMap<bool>> sys::getHostCPUFeatures() { return {}; }
#endif
#if __APPLE__
>From daa180e116d9ae0d78613c109ceff5a85337f5be Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Fri, 5 Jul 2024 14:31:24 +0000
Subject: [PATCH 2/3] Use const and contains
---
lldb/utils/lit-cpuid/lit-cpuid.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lldb/utils/lit-cpuid/lit-cpuid.cpp b/lldb/utils/lit-cpuid/lit-cpuid.cpp
index 16743164e6a5d9..5039077a5e7843 100644
--- a/lldb/utils/lit-cpuid/lit-cpuid.cpp
+++ b/lldb/utils/lit-cpuid/lit-cpuid.cpp
@@ -22,13 +22,13 @@ using namespace llvm;
int main(int argc, char **argv) {
#if defined(__i386__) || defined(_M_IX86) || \
defined(__x86_64__) || defined(_M_X64)
- if (std::optional<StringMap<bool>> features =
+ if (const std::optional<StringMap<bool>> features =
sys::getHostCPUFeatures(features)) {
- if ((*features)["sse"])
+ if (features->contains("sse"))
outs() << "sse\n";
- if ((*features)["avx"])
+ if (features->contains("avx"))
outs() << "avx\n";
- if ((*features)["avx512f"])
+ if (features->contains("avx512f"))
outs() << "avx512f\n";
} else
return 1;
>From 69e246b3d9d03bcd7cf75317fcf5016bfbc19814 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Fri, 5 Jul 2024 15:08:00 +0000
Subject: [PATCH 3/3] contains -> lookup
---
lldb/utils/lit-cpuid/lit-cpuid.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lldb/utils/lit-cpuid/lit-cpuid.cpp b/lldb/utils/lit-cpuid/lit-cpuid.cpp
index 5039077a5e7843..d88c2b7515fdee 100644
--- a/lldb/utils/lit-cpuid/lit-cpuid.cpp
+++ b/lldb/utils/lit-cpuid/lit-cpuid.cpp
@@ -24,11 +24,11 @@ int main(int argc, char **argv) {
defined(__x86_64__) || defined(_M_X64)
if (const std::optional<StringMap<bool>> features =
sys::getHostCPUFeatures(features)) {
- if (features->contains("sse"))
+ if (features->lookup("sse"))
outs() << "sse\n";
- if (features->contains("avx"))
+ if (features->lookup("avx"))
outs() << "avx\n";
- if (features->contains("avx512f"))
+ if (features->lookup("avx512f"))
outs() << "avx512f\n";
} else
return 1;
More information about the lldb-commits
mailing list