[PATCH] D85531: [SystemZ/ZOS] Add support for getHostNumPhysicalCores()

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/unittests/Support/Host.cpp:40
     // physical cores, which is currently only supported/tested for
-    // x86_64 Linux and Darwin.
+    // x86_64 Linux and Darwin and for z/OS.
     return (Host.isOSWindows() && llvm_is_multithreaded()) ||
----------------
abhina.sreeskantharajan wrote:
> Minor nit: grammar
The x86_64 part of the sentence is meant to be distributed across "Linux" and "Darwin" and not to z/OS. With the new formulation, x86_64 needs to be added to before "Darwin" to retain the intended meaning.


================
Comment at: llvm/unittests/Support/Host.cpp:46
+            (Host.isPPC64() || Host.isSystemZ()) ||
+           (Host.isSystemZ() && Host.isOSzOS());
   }
----------------
Minor nit: Aside for Windows, where the expression here is not sensitive to the platform architecture, can we structure this to consistently group by architecture? Alternatively, consistently group by OS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85531