[libcxx-commits] [PATCH] D134216: [SystemZ][z/OS] add code for hardware_concurrency()

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

I am extremely concerned by the need to introduce these sorts of arcane implementation details in the library for z/OS, especially since we don't have a clear idea of where these patches are going to stop.

libc++ is not a "low level system library". It's a library layered on top of C libraries and other standard (or mostly standard) APIs like POSIX and others. It's fine to have some amount of low level and platform specific stuff, but we're not going to implement from scratch APIs that should be provided by any decent OS.



================
Comment at: libcxx/include/thread:274
+#ifdef __MVS__
+    _LIBCPP_INLINE_VISIBILITY
+    static unsigned hardware_concurrency_zos() _NOEXCEPT;
----------------
This can't have inline visibility if it's in the built library.


================
Comment at: libcxx/include/thread:275
+    _LIBCPP_INLINE_VISIBILITY
+    static unsigned hardware_concurrency_zos() _NOEXCEPT;
+#endif
----------------
philnik wrote:
> zibi wrote:
> > This could be non-member static function in `ibm` namespace, same as we did `nonosleep()`..
> You have to __uglify the name.
And yeah it shouldn't be in the `thread` class either way.


================
Comment at: libcxx/src/support/ibm/hardware_concurrency_zos.cpp:1
+//===------------------------- hardware_concurrency_zos.cpp----------------------------------===//
+//
----------------
We don't put file names here anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134216