Fix computeHostNumPhysicalCores() for Linux on POWER and Linux on Z

[etiotto]

Summary

ThinLTO is run using a single thread on Linux on Power. The compute_thread_count() routine calls getHostNumPhysicalCores which returns -1 by default, and so `MaxThreadCount is set to 1.

unsigned llvm::ThreadPoolStrategy::compute_thread_count() const {
   
    int MaxThreadCount = UseHyperThreads ? computeHostNumHardwareThreads()
                                        : sys::getHostNumPhysicalCores();
     if (MaxThreadCount <= 0)
        MaxThreadCount = 1;
   …
}

Fix: provide custom implementation of getHostNumPhysicalCores for Linux on Power and Linux on Z.

[Kai]

LGTM.

[etiotto]

Fix formatting

[jsji]

LoZ.

[uweigand]

LGTM for SystemZ as well.

[etiotto]

Fix test case for PPC64 and SystemZ

[etiotto]

@uweigand and @Kai I forgot to fix a test case. Now fixed are you still ok with the test case change?

[uweigand]

Sure, of course assuming the test still passes.

[MaskRay]

Fix: provide custom implementation of getHostNumPhysicalCores for Linux on Power and Linux on Z.

computeHostNumPhysicalCores() is designed to respect CPU affinity.
SupportTests Threading.PhysicalConcurrency may fail if taskset -c is specified.

I fixed the test in cd53ded557c3487b8dae2f9de894fdb5b75cb8c8. Please refer to the x86 implementation if you want to support both hyper threading & affinity.

llvm/lib/Support/Host.cpp

1275

defined(__ppc__) || defined(__powerpc__) is redundant.

You could just use defined(__powerpc__)

llvm/unittests/Support/Host.cpp

45

Did you want to enable powerpc32 as well?

[etiotto]

llvm/lib/Support/Host.cpp

1275

Other PPC code already uses both. I used both for consistency.

llvm/unittests/Support/Host.cpp

45

No

[MaskRay]

llvm/lib/Support/Host.cpp

1275

This seems like cargo culting. One macro is sufficient.

llvm/unittests/Support/Host.cpp

45

If no, the macro you should have used is __powerpc64__

[RKSimon]

@etiotto @MaskRay This has been failing for a number of days:

http://lab.llvm.org:8011/builders/clang-ppc64le-linux-multistage/builds/12748
http://lab.llvm.org:8011/builders/clang-ppc64le-linux-multistage

Please can somebody take a look and either revert the patch or fix it?

[bmahjour]

@nemanjai has kindly provided a fix https://reviews.llvm.org/rG62a933b72c5b060bcb2c7332d05082f002d6c65a.

[RKSimon]

In D84764#2193380, @bmahjour wrote:

@nemanjai has kindly provided a fix https://reviews.llvm.org/rG62a933b72c5b060bcb2c7332d05082f002d6c65a.

All green - thank you!

llvm/lib/Support/Host.cpp

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(__ppc__) || defined(__powerpc__) || defined(__s390x__)))
+#include <unistd.h>
+
+// Gets the number of *physical cores* on the machine.
+int computeHostNumPhysicalCores() { return sysconf(_SC_NPROCESSORS_ONLN); }
 #elif defined(__APPLE__) && defined(__x86_64__)
 #include <sys/param.h>
 #include <sys/sysctl.h>
 
 // Gets the number of *physical cores* on the machine.
 int computeHostNumPhysicalCores() {
   uint32_t count;
   size_t len = sizeof(count);

llvm/unittests/Support/Host.cpp

 
 protected:
   bool isSupportedArchAndOS() {
     // Initially this is only testing detection of the number of
     // physical cores, which is currently only supported/tested for
     // x86_64 Linux and Darwin.
     return (Host.isOSWindows() && llvm_is_multithreaded()) ||
            (Host.isX86() &&
-            (Host.isOSDarwin() || Host.getOS() == Triple::Linux));
+            (Host.isOSDarwin() || Host.getOS() == Triple::Linux)) ||
+           (Host.getOS() == Triple::Linux &&
+            (Host.isPPC64() || Host.isSystemZ()));
   }
 
   HostTest() : Host(Triple::normalize(sys::getProcessTriple())) {}
 };
 
 TEST_F(HostTest, NumPhysicalCores) {
   int Num = sys::getHostNumPhysicalCores();