[libcxx-commits] [PATCH] D87940: [SystemZ][ZOS] Porting the time functions within libc++ to z/OS

[Zbigniew Sarbinowski via Phabricator via libcxx-commits]

zibi planned changes to this revision.
zibi added a comment.

@ldionne Louis please have a look again. I was not sure if you got the notification.



================
Comment at: libcxx/include/__threading_support:539
    __libcpp_timespec_t __ts = __thread_detail::__convert_to_timespec(__ns);
+#if defined(__MVS__)
+   // The nanosleep() is not available on z/OS. Therefore,
----------------
ldionne wrote:
> Can z/OS implement `nanosleep()` like this, then? Sounds better to implement it once in your system library than in all the places it might be used.
Yes, this is our goal to eventually have nanosleep() part of the OS and eventually remove this workaround.


================
Comment at: libcxx/src/filesystem/filesystem_common.h:411
+#if defined(__MVS__)
+  struct ::timeval ConvertedTS[2] = {
+      {TS[0].tv_sec, {}, Convert(TS[0].tv_nsec)},
----------------
ldionne wrote:
> So IIUC, `timeval` on z/OS is non-conforming to https://pubs.opengroup.org/onlinepubs/007908775/xsh/systime.h.html?
> 
> If we start having such differences, I think it would be better to introduce a `__libcpp_timeval` typedef and a `__libcpp_make_timeval(sec, nsec)` function. We have similar "abstraction" layers in the threading library.
I will experiment with your suggestion and post a new patch if successful. 

As for the non-conforming I'm not sure because z/OS do provide both tv_sev and tv_usec.  It all boils down to the underlying type of suseconds_t.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87940