[libcxx-commits] [PATCH] D93542: [SystemZ][ZOS] Provide CLOCK_MONOTONIC alternative

[Zibi Sarbino via Phabricator via libcxx-commits]

zibi marked 10 inline comments as done.
zibi added a comment.

Thank you louis and Hubert for review and suggestions.



================
Comment at: libcxx/CMakeLists.txt:536
+  if (ZOS)
+    add_definitions(-D_LARGE_TIME_API) # Needed for struct timespec64.
+  endif()
----------------
hubert.reinterpretcast wrote:
> ldionne wrote:
> > Can you bury this into `gettod.h`?
> That may be insufficient if the system headers with the affected interfaces get included before `gettod.h` is reached.
This has to be defined here so the macro is defined before any system headers is included as stated in the comment. We took this same approach in clang code were similar macros needed to be defined with same requirements.


================
Comment at: libcxx/include/support/ibm/gettod.h:13-17
+#include <assert.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <time.h>
----------------
hubert.reinterpretcast wrote:
> This seems to be a lot of header includes to get the definition of `timespec64`. I don't see a use of `assert` in the code.
The original code was reduced which does not require all the header.  Let me prune the list.


================
Comment at: libcxx/include/support/ibm/gettod.h:19
+
+inline int __gettod(struct timespec64 *output) {
+
----------------
ldionne wrote:
> Can you please use a name that conveys more meaning? Something like `gettimeofday_monotonic` maybe?
sure


================
Comment at: libcxx/include/support/ibm/gettod.h:26
+   struct _t {
+      unsigned long hi;
+      unsigned long lo;
----------------
hubert.reinterpretcast wrote:
> Are the `unsigned long`s in this file supposed to differ in size by address mode? If not, I suggest using the uint//N//_t typedefs.
Thanks, what will help with 32-bit run time down the road.


================
Comment at: libcxx/include/support/ibm/gettod.h:41
+   ms = ms - 2208988800000000;
+   output->tv_sec = ms / 1000000;
+   output->tv_nsec = (ms % 1000000) * 1000 + ns;
----------------
hubert.reinterpretcast wrote:
> Would `lldiv` help here?
Will use HLASM dlgr to get better performance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93542