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

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added inline comments.


================
Comment at: llvm/lib/Support/Host.cpp:1334
+      static_cast<uintptr_t>(reinterpret_cast<unsigned int &>(CVT[CVTCSD])));
+  return reinterpret_cast<int &>(CSD[CSD_NUMBER_ONLINE_STANDARD_CPS]);
+}
----------------
hubert.reinterpretcast wrote:
> aganea wrote:
> > hubert.reinterpretcast wrote:
> > > aganea wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > Kai wrote:
> > > > > > MaskRay wrote:
> > > > > > > If z/OS supports something similar to `taskset -c 0-3 $program`, make sure the function returns a number less than or equal to 4. 
> > > > > > Thanks for the hint. This kind of command is not supported on z/OS.
> > > > > > The reason is the fault tolerance available in z/OS: if a defect in a CP is detected, then this CP can go offline and a spare CP can continue to run the code exactly at the point where the former CP went offline. This does not work if the process is tied to a certain set of CPs, and therefore it's not supported.
> > > > > @MaskRay, I think the test you propose imposes an additional restriction that I am not sure the existing implementations of the function adhere to. Namely, I believe the test that you proposed indicates that the intent is to return a number that is one greater than the highest processor index number on the system. I think this might differ from the count of available processors. My understanding is that the function is meant to return the latter.
> > > > @hubert.reinterpretcast Actually both the Linux & Windows implementations return the restricted set of CPUs/processors that are usable within the process (ie. CPUs represented by the affinity mask).
> > > > https://github.com/llvm/llvm-project/blob/62a933b72c5b060bcb2c7332d05082f002d6c65a/llvm/lib/Support/Host.cpp#L1234
> > > The restricted set is what I meant by "available". In other words, I understand that the current implementations returns what you describe.
> > > 
> > > My comment was on the relationship between the count and the index. If the first usable index is higher than the count, then the command MaskRay proposed would fail.
> > I think @MaskRay was only suggesting to take the affinity mask into account. On Linux you could say `taskset -c 6-8,10,15` and in that case `computeHostNumPhysicalCores()` would return 5. Or maybe I'm misunderstanding to which count and index are you referring to?
> Your example is more clear (not fixed to `0` on one end). If that's what @MaskRay meant, then I agree.
Sorry wrong example, that should return 4, as CPU 6 & 7 should be on the same core on a SMT system, but you got the idea.


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

https://reviews.llvm.org/D85531