[PATCH] D84764: Fix computeHostNumPhysicalCores() for Linux on POWER and Linux on Z

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Support/Host.cpp:1275
+#elif (defined(__linux__) &&                                                   \
+       (defined(__ppc__) || defined(__powerpc__) || defined(__s390x__)))
+#include <unistd.h>
----------------
etiotto wrote:
> MaskRay wrote:
> > `defined(__ppc__) || defined(__powerpc__) ` is redundant. 
> > 
> > You could just use `defined(__powerpc__)`
> Other PPC code already uses both. I used both for consistency.
This seems like cargo culting. One macro is sufficient.


================
Comment at: llvm/unittests/Support/Host.cpp:45
+           (Host.getOS() == Triple::Linux &&
+            (Host.isPPC64() || Host.isSystemZ()));
   }
----------------
etiotto wrote:
> MaskRay wrote:
> > Did you want to enable powerpc32 as well?
> No
If no, the macro you should have used is `__powerpc64__`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84764/new/

https://reviews.llvm.org/D84764