[Support] Move getHostNumPhysicalCores to Threading.h

[lenary]

Summary

This change is focussed on simplifying Support/Host.h to only do
target detection. In this case, this function is close in usage to
existing functions in Support/Threading.h, so I moved it into there.
The function is also renamed to llvm::get_physical_cores() to match
the style of threading's functions.

The big change here is that now if you have threading disabled,
llvm::get_physical_cores() will return -1, as if it had not been able
to work out the right info. This is due to how Threading.cpp includes
OS-specific code/headers. This seems ok, as if threading is disabled,
LLVM should not need to know the number of physical cores.

[tmatheson]

Makes sense since it is so similar to get_cpus() and is only used in Threading.

[fpetrogalli]

llvm/unittests/Support/Host.cpp

33

Is this needed?

401

I think you can remove the class HostTest and convert the test fixture into just TEST( instead of TEST_F(. It is kind of awkward to have this: class HostTest : public testing::Test {};

llvm/unittests/Support/Threading.cpp

30

I don't think you need to derive from testing:Test? The functionality of `isSupportedArchAndOS could be achieved just with a function.

[lenary]

llvm/unittests/Support/Host.cpp

33

My original intention was to do the minimal changes required to just move the code.

It's not technically needed, so I will remove it.

401

Yeah, Will do.

llvm/unittests/Support/Threading.cpp

30

Yes, will do.

[fpetrogalli]

llvm/unittests/Support/Host.cpp

33

Yeah, I totally get the idea of doing minimal changes. Juts mention in the commit message that you have some extra cleanup that is not strictly needed for the major changes of the patch, so that people blaming the change can understand why that was done.

[lenary]

llvm/unittests/Support/Host.cpp

33

So, I forgot to submit this comment *before* I submitted the changes. If you're happy with how the patch looks now, +2 and I'll land it.

[fpetrogalli]

LGTM. Thank you @lenary

[fhahn]

Unfortunately it looks like this commit breaks building on ARM64 macOS. I reverted the change for now and added more details on the error in the revert commit.

[lenary]

In D137836#3950815, @fhahn wrote:

Unfortunately it looks like this commit breaks building on ARM64 macOS. I reverted the change for now and added more details on the error in the revert commit.

Thanks for reverting. I hadn't seen any buildbot messages about this until now. It's annoying there's no mac pre-merge testing.

[fhahn]

In D137836#3950825, @lenary wrote:

In D137836#3950815, @fhahn wrote:

Unfortunately it looks like this commit breaks building on ARM64 macOS. I reverted the change for now and added more details on the error in the revert commit.

Thanks for reverting. I hadn't seen any buildbot messages about this until now. It's annoying there's no mac pre-merge testing.

FYI macOS testing is happening on https://green.lab.llvm.org. The failing build is https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/32875/console. From the log it looks like the bot didn't send an email though. Just to double check, you didn't receive an email notification about this?

[fhahn]

(it looks like this job should have sent an email: https://green.lab.llvm.org/green/job/clang-stage1-RA/32031/console)

[lenary]

In D137836#3950846, @fhahn wrote:

(it looks like this job should have sent an email: https://green.lab.llvm.org/green/job/clang-stage1-RA/32031/console)

Yeah, sorry, no email. I'll check with colleagues to see if other people do get emails from this CI instance.

[fhahn]

In D137836#3950883, @lenary wrote:

In D137836#3950846, @fhahn wrote:

(it looks like this job should have sent an email: https://green.lab.llvm.org/green/job/clang-stage1-RA/32031/console)

Yeah, sorry, no email. I'll check with colleagues to see if other people do get emails from this CI instance.

Thanks for confirming, I'll let the maintainers know.

[lenary]

New version, which changes how the move works a bit. I've updated the description with the caveats, so this is now definitely not NFC.

I'm looking for re-review, and maybe input from @MaskRay who has made some minor cleanups in this code recently.

[tmatheson]

One thought about returning -1 but otherwise LGTM

llvm/lib/Support/Threading.cpp

60

It looks like this is the only place get_physical_cores is used, and if the number is unknown or threading is disabled MaxThreadCount is just set to 1. Would it not make sense to change get_physical_cores to return 1, like compute_thread_count?

[lenary]

Going to attempt to land this again today.

llvm/lib/Support/Threading.cpp

60

I'd rather keep the unknown semantic, so it's clearer why someone is getting the answer they are.

[DavidSpickett]

Looks like tests need updating for that new -1 return value: https://lab.llvm.org/buildbot/#/builders/178/builds/3419

That bot builds with threading disabled.

[lenary]

In D137836#3959693, @DavidSpickett wrote:

Looks like tests need updating for that new -1 return value: https://lab.llvm.org/buildbot/#/builders/178/builds/3419

That bot builds with threading disabled.

Thanks for letting me know. I'm thinking about the fix, likely an update to isThreadingSupportedArchAndOs.

[lenary]

In D137836#3959759, @lenary wrote:

In D137836#3959693, @DavidSpickett wrote:

Looks like tests need updating for that new -1 return value: https://lab.llvm.org/buildbot/#/builders/178/builds/3419

That bot builds with threading disabled.

Thanks for letting me know. I'm thinking about the fix, likely an update to isThreadingSupportedArchAndOs.

https://reviews.llvm.org/D139015 Patch here

clang-tools-extra/clangd/test/Inputs/BenchmarkHeader.h

 namespace clang {
 namespace clangd {
 namespace dex {
 class Dex;
 } // namespace dex
 } // namespace clangd
 } // namespace clang
 
 namespace llvm {
-namespace sys {
-
-int getHostNumPhysicalCores();
-
-} // namespace sys
+int get_physical_cores();
 } // namespace llvm
 
 namespace {
 int Variable;
 } // namespace

llvm/include/llvm/Support/Host.h

namespace sys {
   /// \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);
 
-  /// Get the number of physical cores (as opposed to logical cores returned
-  /// from thread::hardware_concurrency(), which includes hyperthreads).
-  /// Returns -1 if unknown for the current host system.
-  int getHostNumPhysicalCores();
-
   namespace detail {
   /// Helper functions to extract HostCPUName from /proc/cpuinfo on linux.
   StringRef getHostCPUNameForPowerPC(StringRef ProcCpuinfoContent);
   StringRef getHostCPUNameForARM(StringRef ProcCpuinfoContent);
   StringRef getHostCPUNameForS390x(StringRef ProcCpuinfoContent);
   StringRef getHostCPUNameForRISCV(StringRef ProcCpuinfoContent);
   StringRef getHostCPUNameForSPARC(StringRef ProcCpuinfoContent);
   StringRef getHostCPUNameForBPF();

llvm/include/llvm/Support/Threading.h

#endif
   /// Returns a mask that represents on which hardware thread, core, CPU, NUMA
   /// group, the calling thread can be executed. On Windows, threads cannot
   /// cross CPU sockets boundaries.
   llvm::BitVector get_thread_affinity_mask();
 
   /// Returns how many physical CPUs or NUMA groups the system has.
   unsigned get_cpus();
 
+  /// Returns how many physical cores (as opposed to logical cores returned from
+  /// thread::hardware_concurrency(), which includes hyperthreads).
+  /// Returns -1 if unknown for the current host system.
+  int get_physical_cores();
+
   enum class ThreadPriority {
     /// Lower the current thread's priority as much as possible. Can be used
     /// for long-running tasks that are not time critical; more energy-
     /// efficient than Low.
     Background = 0,
 
     /// Lower the current thread's priority such that it does not affect
     /// foreground tasks significantly. This is a good default for long-

llvm/lib/Support/Host.cpp

 }
 
 } // namespace x86
 } // namespace detail
 } // namespace sys
 } // namespace llvm
 #endif
 
-#if defined(__linux__) && (defined(__i386__) || defined(__x86_64__))
-// On Linux, the number of physical cores can be computed from /proc/cpuinfo,
-// using the number of unique physical/core id pairs. The following
-// implementation reads the /proc/cpuinfo format on an x86_64 system.
-int computeHostNumPhysicalCores() {
-  // Enabled represents the number of physical id/core id pairs with at least
-  // one processor id enabled by the CPU affinity mask.
-  cpu_set_t Affinity, Enabled;
-  if (sched_getaffinity(0, sizeof(Affinity), &Affinity) != 0)
-    return -1;
-  CPU_ZERO(&Enabled);
-
-  // Read /proc/cpuinfo as a stream (until EOF reached). It cannot be
-  // mmapped because it appears to have 0 size.
-  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text =
-      llvm::MemoryBuffer::getFileAsStream("/proc/cpuinfo");
-  if (std::error_code EC = Text.getError()) {
-    llvm::errs() << "Can't read "
-                 << "/proc/cpuinfo: " << EC.message() << "\n";
-    return -1;
-  }
-  SmallVector<StringRef, 8> strs;
-  (*Text)->getBuffer().split(strs, "\n", /*MaxSplit=*/-1,
-                             /*KeepEmpty=*/false);
-  int CurProcessor = -1;
-  int CurPhysicalId = -1;
-  int CurSiblings = -1;
-  int CurCoreId = -1;
-  for (StringRef Line : strs) {
-    std::pair<StringRef, StringRef> Data = Line.split(':');
-    auto Name = Data.first.trim();
-    auto Val = Data.second.trim();
-    // These fields are available if the kernel is configured with CONFIG_SMP.
-    if (Name == "processor")
-      Val.getAsInteger(10, CurProcessor);
-    else if (Name == "physical id")
-      Val.getAsInteger(10, CurPhysicalId);
-    else if (Name == "siblings")
-      Val.getAsInteger(10, CurSiblings);
-    else if (Name == "core id") {
-      Val.getAsInteger(10, CurCoreId);
-      // The processor id corresponds to an index into cpu_set_t.
-      if (CPU_ISSET(CurProcessor, &Affinity))
-        CPU_SET(CurPhysicalId * CurSiblings + CurCoreId, &Enabled);
-    }
-  }
-  return CPU_COUNT(&Enabled);
-}
-#elif defined(__linux__) && defined(__s390x__)
-int computeHostNumPhysicalCores() { return sysconf(_SC_NPROCESSORS_ONLN); }
-#elif defined(__linux__) && !defined(__ANDROID__)
-int computeHostNumPhysicalCores() {
-  cpu_set_t Affinity;
-  if (sched_getaffinity(0, sizeof(Affinity), &Affinity) == 0)
-    return CPU_COUNT(&Affinity);
-
-  // The call to sched_getaffinity() may have failed because the Affinity
-  // mask is too small for the number of CPU's on the system (i.e. the
-  // system has more than 1024 CPUs). Allocate a mask large enough for
-  // twice as many CPUs.
-  cpu_set_t *DynAffinity;
-  DynAffinity = CPU_ALLOC(2048);
-  if (sched_getaffinity(0, CPU_ALLOC_SIZE(2048), DynAffinity) == 0) {
-    int NumCPUs = CPU_COUNT(DynAffinity);
-    CPU_FREE(DynAffinity);
-    return NumCPUs;
-  }
-  return -1;
-}
-#elif defined(__APPLE__)
-// Gets the number of *physical cores* on the machine.
-int computeHostNumPhysicalCores() {
-  uint32_t count;
-  size_t len = sizeof(count);
-  sysctlbyname("hw.physicalcpu", &count, &len, NULL, 0);
-  if (count < 1) {
-    int nm[2];
-    nm[0] = CTL_HW;
-    nm[1] = HW_AVAILCPU;
-    sysctl(nm, 2, &count, &len, NULL, 0);
-    if (count < 1)
-      return -1;
-  }
-  return count;
-}
-#elif defined(__MVS__)
-int computeHostNumPhysicalCores() {
-  enum {
-    // Byte offset of the pointer to the Communications Vector Table (CVT) in
-    // the Prefixed Save Area (PSA). The table entry is a 31-bit pointer and
-    // will be zero-extended to uintptr_t.
-    FLCCVT = 16,
-    // Byte offset of the pointer to the Common System Data Area (CSD) in the
-    // CVT. The table entry is a 31-bit pointer and will be zero-extended to
-    // uintptr_t.
-    CVTCSD = 660,
-    // Byte offset to the number of live CPs in the LPAR, stored as a signed
-    // 32-bit value in the table.
-    CSD_NUMBER_ONLINE_STANDARD_CPS = 264,
-  };
-  char *PSA = 0;
-  char *CVT = reinterpret_cast<char *>(
-      static_cast<uintptr_t>(reinterpret_cast<unsigned int &>(PSA[FLCCVT])));
-  char *CSD = reinterpret_cast<char *>(
-      static_cast<uintptr_t>(reinterpret_cast<unsigned int &>(CVT[CVTCSD])));
-  return reinterpret_cast<int &>(CSD[CSD_NUMBER_ONLINE_STANDARD_CPS]);
-}
-#elif defined(_WIN32) && LLVM_ENABLE_THREADS != 0
-// Defined in llvm/lib/Support/Windows/Threading.inc
-int computeHostNumPhysicalCores();
-#else
-// On other systems, return -1 to indicate unknown.
-static int computeHostNumPhysicalCores() { return -1; }
-#endif
-
-int sys::getHostNumPhysicalCores() {
-  static int NumCores = computeHostNumPhysicalCores();
-  return NumCores;
-}
-
 #if defined(__i386__) || defined(_M_IX86) || \
     defined(__x86_64__) || defined(_M_X64)
 bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
   unsigned EAX = 0, EBX = 0, ECX = 0, EDX = 0;
   unsigned MaxLevel;
 
   if (getX86CpuIDAndInfo(0, &MaxLevel, &EBX, &ECX, &EDX) || MaxLevel < 1)
     return false;

llvm/lib/Support/Threading.cpp

 //===-- llvm/Support/Threading.cpp- Control multithreading mode --*- C++ -*-==//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines helper functions for running LLVM in a multi-threaded
 // environment.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/Threading.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Config/config.h"
-#include "llvm/Support/Host.h"
+#include "llvm/Config/llvm-config.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/raw_ostream.h"
 
 #include <cassert>
 #include <errno.h>
 #include <stdlib.h>
 #include <string.h>
 
 using namespace llvm;
 
unsigned llvm::ThreadPoolStrategy::compute_thread_count() const {
   return 1;
 }
 
 #else
 
 int computeHostNumHardwareThreads();
 
 unsigned llvm::ThreadPoolStrategy::compute_thread_count() const {
-  int MaxThreadCount = UseHyperThreads ? computeHostNumHardwareThreads()
-                                       : sys::getHostNumPhysicalCores();
+  int MaxThreadCount =
+      UseHyperThreads ? computeHostNumHardwareThreads() : get_physical_cores();
   if (MaxThreadCount <= 0)
     MaxThreadCount = 1;
   if (ThreadsRequested == 0)
     return MaxThreadCount;
   if (!Limit)
     return ThreadsRequested;
   return std::min((unsigned)MaxThreadCount, ThreadsRequested);
 }
 
 // Include the platform-specific parts of this class.
llvm::get_threadpool_strategy(StringRef Num, ThreadPoolStrategy Default) {
 
   // Do not take the Default into account. This effectively disables
   // heavyweight_hardware_concurrency() if the user asks for any number of
   // threads on the cmd-line.
   ThreadPoolStrategy S = llvm::hardware_concurrency();
   S.ThreadsRequested = V;
   return S;
 }
+
+#if defined(__linux__) && (defined(__i386__) || defined(__x86_64__))
+// On Linux, the number of physical cores can be computed from /proc/cpuinfo,
+// using the number of unique physical/core id pairs. The following
+// implementation reads the /proc/cpuinfo format on an x86_64 system.
+int computeHostNumPhysicalCores() {
+  // Enabled represents the number of physical id/core id pairs with at least
+  // one processor id enabled by the CPU affinity mask.
+  cpu_set_t Affinity, Enabled;
+  if (sched_getaffinity(0, sizeof(Affinity), &Affinity) != 0)
+    return -1;
+  CPU_ZERO(&Enabled);
+
+  // Read /proc/cpuinfo as a stream (until EOF reached). It cannot be
+  // mmapped because it appears to have 0 size.
+  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Text =
+      llvm::MemoryBuffer::getFileAsStream("/proc/cpuinfo");
+  if (std::error_code EC = Text.getError()) {
+    llvm::errs() << "Can't read "
+                 << "/proc/cpuinfo: " << EC.message() << "\n";
+    return -1;
+  }
+  SmallVector<StringRef, 8> strs;
+  (*Text)->getBuffer().split(strs, "\n", /*MaxSplit=*/-1,
+                             /*KeepEmpty=*/false);
+  int CurProcessor = -1;
+  int CurPhysicalId = -1;
+  int CurSiblings = -1;
+  int CurCoreId = -1;
+  for (StringRef Line : strs) {
+    std::pair<StringRef, StringRef> Data = Line.split(':');
+    auto Name = Data.first.trim();
+    auto Val = Data.second.trim();
+    // These fields are available if the kernel is configured with CONFIG_SMP.
+    if (Name == "processor")
+      Val.getAsInteger(10, CurProcessor);
+    else if (Name == "physical id")
+      Val.getAsInteger(10, CurPhysicalId);
+    else if (Name == "siblings")
+      Val.getAsInteger(10, CurSiblings);
+    else if (Name == "core id") {
+      Val.getAsInteger(10, CurCoreId);
+      // The processor id corresponds to an index into cpu_set_t.
+      if (CPU_ISSET(CurProcessor, &Affinity))
+        CPU_SET(CurPhysicalId * CurSiblings + CurCoreId, &Enabled);
+    }
+  }
+  return CPU_COUNT(&Enabled);
+}
+#elif defined(__linux__) && defined(__s390x__)
+int computeHostNumPhysicalCores() { return sysconf(_SC_NPROCESSORS_ONLN); }
+#elif defined(__linux__) && !defined(__ANDROID__)
+int computeHostNumPhysicalCores() {
+  cpu_set_t Affinity;
+  if (sched_getaffinity(0, sizeof(Affinity), &Affinity) == 0)
+    return CPU_COUNT(&Affinity);
+
+  // The call to sched_getaffinity() may have failed because the Affinity
+  // mask is too small for the number of CPU's on the system (i.e. the
+  // system has more than 1024 CPUs). Allocate a mask large enough for
+  // twice as many CPUs.
+  cpu_set_t *DynAffinity;
+  DynAffinity = CPU_ALLOC(2048);
+  if (sched_getaffinity(0, CPU_ALLOC_SIZE(2048), DynAffinity) == 0) {
+    int NumCPUs = CPU_COUNT(DynAffinity);
+    CPU_FREE(DynAffinity);
+    return NumCPUs;
+  }
+  return -1;
+}
+#elif defined(__APPLE__)
+// Gets the number of *physical cores* on the machine.
+int computeHostNumPhysicalCores() {
+  uint32_t count;
+  size_t len = sizeof(count);
+  sysctlbyname("hw.physicalcpu", &count, &len, NULL, 0);
+  if (count < 1) {
+    int nm[2];
+    nm[0] = CTL_HW;
+    nm[1] = HW_AVAILCPU;
+    sysctl(nm, 2, &count, &len, NULL, 0);
+    if (count < 1)
+      return -1;
+  }
+  return count;
+}
+#elif defined(__MVS__)
+int computeHostNumPhysicalCores() {
+  enum {
+    // Byte offset of the pointer to the Communications Vector Table (CVT) in
+    // the Prefixed Save Area (PSA). The table entry is a 31-bit pointer and
+    // will be zero-extended to uintptr_t.
+    FLCCVT = 16,
+    // Byte offset of the pointer to the Common System Data Area (CSD) in the
+    // CVT. The table entry is a 31-bit pointer and will be zero-extended to
+    // uintptr_t.
+    CVTCSD = 660,
+    // Byte offset to the number of live CPs in the LPAR, stored as a signed
+    // 32-bit value in the table.
+    CSD_NUMBER_ONLINE_STANDARD_CPS = 264,
+  };
+  char *PSA = 0;
+  char *CVT = reinterpret_cast<char *>(
+      static_cast<uintptr_t>(reinterpret_cast<unsigned int &>(PSA[FLCCVT])));
+  char *CSD = reinterpret_cast<char *>(
+      static_cast<uintptr_t>(reinterpret_cast<unsigned int &>(CVT[CVTCSD])));
+  return reinterpret_cast<int &>(CSD[CSD_NUMBER_ONLINE_STANDARD_CPS]);
+}
+#elif defined(_WIN32) && LLVM_ENABLE_THREADS != 0
+// Defined in llvm/lib/Support/Windows/Threading.inc
+int computeHostNumPhysicalCores();
+#else
+// On other systems, return -1 to indicate unknown.
+static int computeHostNumPhysicalCores() { return -1; }
+#endif
+
+int llvm::get_physical_cores() {
+  static int NumCores = computeHostNumPhysicalCores();
+  return NumCores;
+}

llvm/unittests/Support/Host.cpp

Message << #x ": did not return errc::success.\n"                          \
             << "error number: " << ASSERT_NO_ERROR_ec.value() << "\n"          \
             << "error message: " << ASSERT_NO_ERROR_ec.message() << "\n";      \
     GTEST_FATAL_FAILURE_(MessageStorage.c_str());                              \
   } else {                                                                     \
   }
 
 using namespace llvm;
 
-class HostTest : public testing::Test {
-  Triple Host;
-
-protected:
-  bool isSupportedArchAndOS() {
-    // Initially this is only testing detection of the number of
-    // physical cores, which is currently only supported/tested on
-    // some systems.
-    return (Host.isOSWindows() && llvm_is_multithreaded()) ||
-           Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
-           (Host.isOSLinux() && !Host.isAndroid()) ||
-           (Host.isSystemZ() && Host.isOSzOS());
-  }
-
-  HostTest() : Host(Triple::normalize(sys::getProcessTriple())) {}
-};
-
-TEST_F(HostTest, NumPhysicalCoresSupported) {
-  if (!isSupportedArchAndOS())
-    GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_GT(Num, 0);
-}
-
-TEST_F(HostTest, NumPhysicalCoresUnsupported) {
-  if (isSupportedArchAndOS())
-    GTEST_SKIP();
-  int Num = sys::getHostNumPhysicalCores();
-  ASSERT_EQ(Num, -1);
-}
+class HostTest : public testing::Test {};
 
 TEST(getLinuxHostCPUName, ARM) {
   StringRef CortexA9ProcCpuinfo = R"(
 processor       : 0
 model name      : ARMv7 Processor rev 10 (v7l)
 BogoMIPS        : 1393.66
 Features        : half thumb fastmult vfp edsp thumbee neon vfpv3 tls vfpd32
 CPU implementer : 0x41
[ExePath, argv, &Buffer, &Size, &Success] {
 
     ASSERT_NO_ERROR(fs::remove(OutputPath));
     ASSERT_NO_ERROR(fs::remove(TestDirectory.str()));
     Success = true;
   }();
   return Success;
 }
 
 TEST_F(HostTest, DummyRunAndGetCommandOutputUse) {
   // Suppress defined-but-not-used warnings when the tests using the helper are
   // disabled.
   (void)&runAndGetCommandOutput;
 }
 
 TEST_F(HostTest, getMacOSHostVersion) {
   llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (!HostTriple.isMacOSX())

llvm/unittests/Support/Threading.cpp

 //===- unittests/Threading.cpp - Thread tests -----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/Threading.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/Support/Host.h"
 #include "llvm/Support/thread.h"
 #include "gtest/gtest.h"
 
 #include <atomic>
 #include <condition_variable>
 
 using namespace llvm;
 
 namespace {
 
 TEST(Threading, PhysicalConcurrency) {
   auto Num = heavyweight_hardware_concurrency();
   // Since Num is unsigned this will also catch us trying to
   // return -1.
   ASSERT_LE(Num.compute_thread_count(),
             hardware_concurrency().compute_thread_count());
 }
 
+class ThreadingTest : public testing::Test {
+  Triple Host;
+
+protected:
+  bool isSupportedArchAndOS() {
+    // Initially this is only testing detection of the number of
+    // physical cores, which is currently only supported/tested on
+    // some systems.
+    return (Host.isOSWindows() && llvm_is_multithreaded()) ||
+           Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) ||
+           (Host.isOSLinux() && !Host.isAndroid()) ||
+           (Host.isSystemZ() && Host.isOSzOS());
+  }
+
+  ThreadingTest() : Host(Triple::normalize(sys::getProcessTriple())) {}
+};
+
+TEST_F(ThreadingTest, NumPhysicalCoresSupported) {
+  if (!isSupportedArchAndOS())
+    GTEST_SKIP();
+  int Num = get_physical_cores();
+  ASSERT_GT(Num, 0);
+}
+
+TEST_F(ThreadingTest, NumPhysicalCoresUnsupported) {
+  if (isSupportedArchAndOS())
+    GTEST_SKIP();
+  int Num = get_physical_cores();
+  ASSERT_EQ(Num, -1);
+}
+
 #if LLVM_ENABLE_THREADS
 
 class Notification {
 public:
   void notify() {
     {
       std::lock_guard<std::mutex> Lock(M);
       Notified = true;