Skip to content

Commit

Permalink
Reland 293e8fa
Browse files Browse the repository at this point in the history
    [llvm-exegesis] Disable the LBR check on AMD

    https://bugs.llvm.org/show_bug.cgi?id=48918

    The bug reported a hang (or very very slow runtime) on a Zen2. Unfortunately, we don't have the hardware right now to debug it and I was not able to reproduce the bug on a HSW.
    Theory we've got is that the lbr-checking code could be confused on AMD.

    Differential Revision: https://reviews.llvm.org/D97504

New change:
 - Surround usages of x86 helper in llvm-exegesis/X86/Target.cpp with ifdef
 - Fix bug which caused the caller of getVendorSignature to not have a copy of EAX that it expected.
  • Loading branch information
oontvoo committed Mar 5, 2021
1 parent a9ccdfb commit f8b01d5
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 16 deletions.
14 changes: 14 additions & 0 deletions llvm/include/llvm/Support/Host.h
Expand Up @@ -65,6 +65,20 @@ namespace sys {
StringRef getHostCPUNameForARM(StringRef ProcCpuinfoContent);
StringRef getHostCPUNameForS390x(StringRef ProcCpuinfoContent);
StringRef getHostCPUNameForBPF();

/// Helper functions to extract CPU details from CPUID on x86.
namespace x86 {
enum class VendorSignatures {
UNKNOWN,
GENUINE_INTEL,
AUTHENTIC_AMD,
};

/// Returns the host CPU's vendor.
/// MaxLeaf: if a non-nullptr pointer is specified, the EAX value will be
/// assigned to its pointee.
VendorSignatures getVendorSignature(unsigned *MaxLeaf = nullptr);
} // namespace x86
}
}
}
Expand Down
68 changes: 55 additions & 13 deletions llvm/lib/Support/Host.cpp
Expand Up @@ -420,11 +420,6 @@ StringRef sys::detail::getHostCPUNameForBPF() {
#if defined(__i386__) || defined(_M_IX86) || \
defined(__x86_64__) || defined(_M_X64)

enum VendorSignatures {
SIG_INTEL = 0x756e6547 /* Genu */,
SIG_AMD = 0x68747541 /* Auth */
};

// The check below for i386 was copied from clang's cpuid.h (__get_cpuid_max).
// Check motivated by bug reports for OpenSSL crashing on CPUs without CPUID
// support. Consequently, for i386, the presence of CPUID is checked first
Expand Down Expand Up @@ -498,6 +493,42 @@ static bool getX86CpuIDAndInfo(unsigned value, unsigned *rEAX, unsigned *rEBX,
#endif
}

namespace llvm {
namespace sys {
namespace detail {
namespace x86 {

VendorSignatures getVendorSignature(unsigned *MaxLeaf) {
unsigned EAX = 0, EBX = 0, ECX = 0, EDX = 0;
if (MaxLeaf == nullptr)
MaxLeaf = &EAX;
else
*MaxLeaf = 0;

if (!isCpuIdSupported())
return VendorSignatures::UNKNOWN;

if (getX86CpuIDAndInfo(0, MaxLeaf, &EBX, &ECX, &EDX) || *MaxLeaf < 1)
return VendorSignatures::UNKNOWN;

// "Genu ineI ntel"
if (EBX == 0x756e6547 && ECX == 0x6c65746e && EDX == 0x49656e69)
return VendorSignatures::GENUINE_INTEL;

// "Auth enti cAMD"
if (EBX == 0x68747541 && ECX == 0x69746e65 && EDX == 0x444d4163)
return VendorSignatures::AUTHENTIC_AMD;

return VendorSignatures::UNKNOWN;
}

} // namespace x86
} // namespace detail
} // namespace sys
} // namespace llvm

using namespace llvm::sys::detail::x86;

/// getX86CpuIDAndInfoEx - Execute the specified cpuid with subleaf and return
/// the 4 values in the specified arguments. If we can't run cpuid on the host,
/// return true.
Expand Down Expand Up @@ -1095,14 +1126,12 @@ static void getAvailableFeatures(unsigned ECX, unsigned EDX, unsigned MaxLeaf,
}

StringRef sys::getHostCPUName() {
unsigned EAX = 0, EBX = 0, ECX = 0, EDX = 0;
unsigned MaxLeaf, Vendor;

if (!isCpuIdSupported())
unsigned MaxLeaf = 0;
const VendorSignatures Vendor = getVendorSignature(&MaxLeaf);
if (Vendor == VendorSignatures::UNKNOWN)
return "generic";

if (getX86CpuIDAndInfo(0, &MaxLeaf, &Vendor, &ECX, &EDX) || MaxLeaf < 1)
return "generic";
unsigned EAX = 0, EBX = 0, ECX = 0, EDX = 0;
getX86CpuIDAndInfo(0x1, &EAX, &EBX, &ECX, &EDX);

unsigned Family = 0, Model = 0;
Expand All @@ -1117,10 +1146,10 @@ StringRef sys::getHostCPUName() {

StringRef CPU;

if (Vendor == SIG_INTEL) {
if (Vendor == VendorSignatures::GENUINE_INTEL) {
CPU = getIntelProcessorTypeAndSubtype(Family, Model, Features, &Type,
&Subtype);
} else if (Vendor == SIG_AMD) {
} else if (Vendor == VendorSignatures::AUTHENTIC_AMD) {
CPU = getAMDProcessorTypeAndSubtype(Family, Model, Features, &Type,
&Subtype);
}
Expand Down Expand Up @@ -1254,6 +1283,19 @@ StringRef sys::getHostCPUName() {
}
#else
StringRef sys::getHostCPUName() { return "generic"; }
namespace llvm {
namespace sys {
namespace detail {
namespace x86 {

VendorSignatures getVendorSignature(unsigned *MaxLeaf) {
return VendorSignatures::UNKNOWN;
}

} // namespace x86
} // namespace detail
} // namespace sys
} // namespace llvm
#endif

#if defined(__linux__) && (defined(__i386__) || defined(__x86_64__))
Expand Down
19 changes: 16 additions & 3 deletions llvm/tools/llvm-exegesis/lib/X86/Target.cpp
Expand Up @@ -22,6 +22,7 @@
#include "llvm/Support/Errc.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Host.h"

#include <memory>
#include <string>
Expand Down Expand Up @@ -742,13 +743,25 @@ class ExegesisX86Target : public ExegesisTarget {

#if defined(__linux__) && defined(HAVE_LIBPFM) && \
defined(LIBPFM_HAS_FIELD_CYCLES)
// If the kernel supports it, the hardware still may not have it.
return X86LbrCounter::checkLbrSupport();
// FIXME: Fix this.
// https://bugs.llvm.org/show_bug.cgi?id=48918
// For now, only do the check if we see an Intel machine because
// the counter uses some intel-specific magic and it could
// be confuse and think an AMD machine actually has LBR support.
#if defined(__i386__) || defined(_M_IX86) || defined(__x86_64__) || \
defined(_M_X64)
using namespace sys::detail::x86;

if (getVendorSignature() == VendorSignatures::GENUINE_INTEL)
// If the kernel supports it, the hardware still may not have it.
return X86LbrCounter::checkLbrSupport();
#else
llvm_unreachable("Running X86 exegesis on non-X86 target");
#endif
#endif
return llvm::make_error<llvm::StringError>(
"LBR not supported on this kernel and/or platform",
llvm::errc::not_supported);
#endif
}

std::unique_ptr<SavedState> withSavedState() const override {
Expand Down

0 comments on commit f8b01d5

Please sign in to comment.