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

Mon Aug 10 05:08:17 PDT 2020

Kai marked 2 inline comments as done.
Kai 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()) ||
----------------
hubert.reinterpretcast wrote:
> 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.
I hope the new order is better.
As alternative I also thought about removing this part of the comment, as it only rephrases the condition without giving new information.

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

https://reviews.llvm.org/D85531