[libc-commits] [libc] [libc][NFC] adjust time related implementations (PR #91485)

Schrodinger ZHU Yifan via libc-commits libc-commits at lists.llvm.org
Wed May 8 08:05:16 PDT 2024


https://github.com/SchrodingerZhu created https://github.com/llvm/llvm-project/pull/91485

This PR
- Unify the naming style of `clock_getime_impl`.
- Make `clock_getime_impl` a header library for more internal usage. Such APIs do not pollute `errno` so it is more friendly for using inside libc. Implementations of timed locking may be such candidates.
- Adjust dependency accordingly.

>From a11bba401f0f080171495383bb130823fec9359c Mon Sep 17 00:00:00 2001
From: Schrodinger ZHU Yifan <yifanzhu at rochester.edu>
Date: Wed, 8 May 2024 11:01:24 -0400
Subject: [PATCH] [libc][NFC] adjust time related implementations

---
 libc/src/time/linux/CMakeLists.txt            | 26 ++++++++------
 libc/src/time/linux/clock.cpp                 | 17 ++++------
 libc/src/time/linux/clock_gettime.cpp         |  8 ++---
 ...lockGetTimeImpl.h => clock_gettime_impl.h} | 34 +++++++++++++++----
 libc/src/time/linux/gettimeofday.cpp          | 11 +++---
 libc/src/time/linux/time.cpp                  | 12 +++----
 6 files changed, 61 insertions(+), 47 deletions(-)
 rename libc/src/time/linux/{clockGetTimeImpl.h => clock_gettime_impl.h} (60%)

diff --git a/libc/src/time/linux/CMakeLists.txt b/libc/src/time/linux/CMakeLists.txt
index df79bf5986261..e0008ed7cdf1f 100644
--- a/libc/src/time/linux/CMakeLists.txt
+++ b/libc/src/time/linux/CMakeLists.txt
@@ -1,3 +1,13 @@
+add_header_library(
+  clock_gettime_impl
+  HDRS
+    clock_gettime_impl.h
+  DEPENDS
+    libc.include.sys_syscall
+    libc.src.__support.OSUtil.osutil
+    libc.src.__support.error_or
+)
+
 add_entrypoint_object(
   time
   SRCS
@@ -5,9 +15,8 @@ add_entrypoint_object(
   HDRS
     ../time_func.h
   DEPENDS
+    .clock_gettime_impl
     libc.include.time
-    libc.include.sys_syscall
-    libc.src.__support.OSUtil.osutil
     libc.src.errno.errno
 )
 
@@ -18,10 +27,9 @@ add_entrypoint_object(
   HDRS
     ../clock.h
   DEPENDS
+    .clock_gettime_impl
     libc.include.time
-    libc.include.sys_syscall
     libc.src.__support.CPP.limits
-    libc.src.__support.OSUtil.osutil
     libc.src.errno.errno
 )
 
@@ -32,10 +40,10 @@ add_entrypoint_object(
   HDRS
     ../nanosleep.h
   DEPENDS
-    libc.include.time
     libc.include.sys_syscall
-    libc.src.__support.CPP.limits
     libc.src.__support.OSUtil.osutil
+    libc.include.time
+    libc.src.__support.CPP.limits
     libc.src.errno.errno
 )
 
@@ -46,9 +54,8 @@ add_entrypoint_object(
   HDRS
     ../clock_gettime.h
   DEPENDS
+    .clock_gettime_impl
     libc.include.time
-    libc.include.sys_syscall
-    libc.src.__support.OSUtil.osutil
     libc.src.errno.errno
 )
 
@@ -59,8 +66,7 @@ add_entrypoint_object(
   HDRS
     ../gettimeofday.h
   DEPENDS
+    .clock_gettime_impl
     libc.include.time
-    libc.include.sys_syscall
-    libc.src.__support.OSUtil.osutil
     libc.src.errno.errno
 )
diff --git a/libc/src/time/linux/clock.cpp b/libc/src/time/linux/clock.cpp
index 1e95f0526bc9c..39606f6d4e988 100644
--- a/libc/src/time/linux/clock.cpp
+++ b/libc/src/time/linux/clock.cpp
@@ -7,21 +7,18 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/time/clock.h"
-
 #include "src/__support/CPP/limits.h"
-#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/common.h"
 #include "src/errno/libc_errno.h"
-#include "src/time/linux/clockGetTimeImpl.h"
-
-#include <sys/syscall.h> // For syscall numbers.
+#include "src/time/linux/clock_gettime_impl.h"
 #include <time.h>
 
 namespace LIBC_NAMESPACE {
 
 LLVM_LIBC_FUNCTION(clock_t, clock, ()) {
+  using namespace internal::time_units;
   struct timespec ts;
-  auto result = internal::clock_gettimeimpl(CLOCK_PROCESS_CPUTIME_ID, &ts);
+  auto result = internal::clock_gettime_impl(CLOCK_PROCESS_CPUTIME_ID, &ts);
   if (!result.has_value()) {
     libc_errno = result.error();
     return -1;
@@ -34,15 +31,15 @@ LLVM_LIBC_FUNCTION(clock_t, clock, ()) {
       cpp::numeric_limits<clock_t>::max() / CLOCKS_PER_SEC;
   if (ts.tv_sec > CLOCK_SECS_MAX)
     return clock_t(-1);
-  if (ts.tv_nsec / 1000000000 > CLOCK_SECS_MAX - ts.tv_sec)
+  if (ts.tv_nsec / 1_s_ns > CLOCK_SECS_MAX - ts.tv_sec)
     return clock_t(-1);
 
   // For the integer computation converting tv_nsec to clocks to work
   // correctly, we want CLOCKS_PER_SEC to be less than 1000000000.
-  static_assert(1000000000 > CLOCKS_PER_SEC,
-                "Expected CLOCKS_PER_SEC to be less than 1000000000.");
+  static_assert(1_s_ns > CLOCKS_PER_SEC,
+                "Expected CLOCKS_PER_SEC to be less than 1'000'000'000.");
   return clock_t(ts.tv_sec * CLOCKS_PER_SEC +
-                 ts.tv_nsec / (1000000000 / CLOCKS_PER_SEC));
+                 ts.tv_nsec / (1_s_ns / CLOCKS_PER_SEC));
 }
 
 } // namespace LIBC_NAMESPACE
diff --git a/libc/src/time/linux/clock_gettime.cpp b/libc/src/time/linux/clock_gettime.cpp
index 47e974a866c83..f2943955bc78b 100644
--- a/libc/src/time/linux/clock_gettime.cpp
+++ b/libc/src/time/linux/clock_gettime.cpp
@@ -7,13 +7,9 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/time/clock_gettime.h"
-
-#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/common.h"
 #include "src/errno/libc_errno.h"
-#include "src/time/linux/clockGetTimeImpl.h"
-
-#include <sys/syscall.h> // For syscall numbers.
+#include "src/time/linux/clock_gettime_impl.h"
 #include <time.h>
 
 namespace LIBC_NAMESPACE {
@@ -21,7 +17,7 @@ namespace LIBC_NAMESPACE {
 // TODO(michaelrj): Move this into time/linux with the other syscalls.
 LLVM_LIBC_FUNCTION(int, clock_gettime,
                    (clockid_t clockid, struct timespec *ts)) {
-  auto result = internal::clock_gettimeimpl(clockid, ts);
+  auto result = internal::clock_gettime_impl(clockid, ts);
 
   // A negative return value indicates an error with the magnitude of the
   // value being the error code.
diff --git a/libc/src/time/linux/clockGetTimeImpl.h b/libc/src/time/linux/clock_gettime_impl.h
similarity index 60%
rename from libc/src/time/linux/clockGetTimeImpl.h
rename to libc/src/time/linux/clock_gettime_impl.h
index 8c8c9fcf845cc..752434018e261 100644
--- a/libc/src/time/linux/clockGetTimeImpl.h
+++ b/libc/src/time/linux/clock_gettime_impl.h
@@ -6,13 +6,13 @@
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_LIBC_SRC_TIME_LINUX_CLOCKGETTIMEIMPL_H
-#define LLVM_LIBC_SRC_TIME_LINUX_CLOCKGETTIMEIMPL_H
+#ifndef LLVM_LIBC_SRC_TIME_LINUX_CLOCK_GETTIME_IMPL_H
+#define LLVM_LIBC_SRC_TIME_LINUX_CLOCK_GETTIME_IMPL_H
 
 #include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/common.h"
 #include "src/__support/error_or.h"
-#include "src/errno/libc_errno.h"
+#include "src/__support/macros/attributes.h"
 
 #include <stdint.h>      // For int64_t.
 #include <sys/syscall.h> // For syscall numbers.
@@ -21,8 +21,30 @@
 namespace LIBC_NAMESPACE {
 namespace internal {
 
-LIBC_INLINE ErrorOr<int> clock_gettimeimpl(clockid_t clockid,
-                                           struct timespec *ts) {
+namespace time_units {
+LIBC_INLINE constexpr time_t operator""_s_ns(unsigned long long s) {
+  return s * 1'000'000'000;
+}
+LIBC_INLINE constexpr time_t operator""_s_us(unsigned long long s) {
+  return s * 1'000'000;
+}
+LIBC_INLINE constexpr time_t operator""_s_ms(unsigned long long s) {
+  return s * 1'000;
+}
+LIBC_INLINE constexpr time_t operator""_ms_ns(unsigned long long ms) {
+  return ms * 1'000'000;
+}
+LIBC_INLINE constexpr time_t operator""_ms_us(unsigned long long ms) {
+  return ms * 1'000;
+}
+LIBC_INLINE constexpr time_t operator""_us_ns(unsigned long long us) {
+  return us * 1'000;
+}
+} // namespace time_units
+// namespace time_units
+
+LIBC_INLINE ErrorOr<int> clock_gettime_impl(clockid_t clockid,
+                                            struct timespec *ts) {
 #if SYS_clock_gettime
   int ret = LIBC_NAMESPACE::syscall_impl<int>(SYS_clock_gettime,
                                               static_cast<long>(clockid),
@@ -45,4 +67,4 @@ LIBC_INLINE ErrorOr<int> clock_gettimeimpl(clockid_t clockid,
 } // namespace internal
 } // namespace LIBC_NAMESPACE
 
-#endif // LLVM_LIBC_SRC_TIME_LINUX_CLOCKGETTIMEIMPL_H
+#endif // LLVM_LIBC_SRC_TIME_LINUX_CLOCK_GETTIME_IMPL_H
diff --git a/libc/src/time/linux/gettimeofday.cpp b/libc/src/time/linux/gettimeofday.cpp
index 07ab4d579176e..bf388a656441f 100644
--- a/libc/src/time/linux/gettimeofday.cpp
+++ b/libc/src/time/linux/gettimeofday.cpp
@@ -7,24 +7,21 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/time/gettimeofday.h"
-
-#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/common.h"
 #include "src/errno/libc_errno.h"
-#include "src/time/linux/clockGetTimeImpl.h"
-
-#include <sys/syscall.h> // For syscall numbers.
+#include "src/time/linux/clock_gettime_impl.h"
 
 namespace LIBC_NAMESPACE {
 
 // TODO(michaelrj): Move this into time/linux with the other syscalls.
 LLVM_LIBC_FUNCTION(int, gettimeofday,
                    (struct timeval * tv, [[maybe_unused]] void *unused)) {
+  using namespace internal::time_units;
   if (tv == nullptr)
     return 0;
 
   struct timespec ts;
-  auto result = internal::clock_gettimeimpl(CLOCK_REALTIME, &ts);
+  auto result = internal::clock_gettime_impl(CLOCK_REALTIME, &ts);
 
   // A negative return value indicates an error with the magnitude of the
   // value being the error code.
@@ -34,7 +31,7 @@ LLVM_LIBC_FUNCTION(int, gettimeofday,
   }
 
   tv->tv_sec = ts.tv_sec;
-  tv->tv_usec = static_cast<suseconds_t>(ts.tv_nsec / 1000);
+  tv->tv_usec = static_cast<suseconds_t>(ts.tv_nsec / 1_us_ns);
   return 0;
 }
 
diff --git a/libc/src/time/linux/time.cpp b/libc/src/time/linux/time.cpp
index e286fae095b2a..336c723356805 100644
--- a/libc/src/time/linux/time.cpp
+++ b/libc/src/time/linux/time.cpp
@@ -6,22 +6,18 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "src/time/time_func.h"
-
-#include "src/__support/OSUtil/syscall.h" // For internal syscall function.
 #include "src/__support/common.h"
 #include "src/errno/libc_errno.h"
-#include "src/time/linux/clockGetTimeImpl.h"
-
-#include <sys/syscall.h> // For syscall numbers.
+#include "src/time/linux/clock_gettime_impl.h"
+#include "src/time/time_func.h"
 #include <time.h>
 
 namespace LIBC_NAMESPACE {
 
-LLVM_LIBC_FUNCTION(time_t, time, (time_t * tp)) {
+LLVM_LIBC_FUNCTION(time_t, time, (time_t *tp)) {
   // TODO: Use the Linux VDSO to fetch the time and avoid the syscall.
   struct timespec ts;
-  auto result = internal::clock_gettimeimpl(CLOCK_REALTIME, &ts);
+  auto result = internal::clock_gettime_impl(CLOCK_REALTIME, &ts);
   if (!result.has_value()) {
     libc_errno = result.error();
     return -1;



More information about the libc-commits mailing list