[libc-commits] [libc] [libc] Make use of 32-bit time_t a config option (PR #102012)
Simon Tatham via libc-commits
libc-commits at lists.llvm.org
Thu Aug 8 08:13:51 PDT 2024
https://github.com/statham-arm updated https://github.com/llvm/llvm-project/pull/102012
>From fad0d6a3043680273512e4716d6c8a0b45dcde5a Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tatham at arm.com>
Date: Mon, 5 Aug 2024 16:35:41 +0100
Subject: [PATCH 1/3] [libc] Make use of 32-bit time_t a config option
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The 32-bit Arm builds of libc define time_t to be `__INTPTR_TYPE__`,
i.e. a 32-bit integer. This is commented in the commit introducing it
(75398f28ebdb600) as being for compatibility with glibc. But in the
near future not even every AArch32 build of glibc will have a 32-bit
time_t: Debian is planning that their next release (trixie) will have
switched to 64-bit. And non-Linux builds of this libc (e.g. baremetal)
have no reason to need glibc compatibility in the first place – and
every reason _not_ to want to start using a 32-bit time_t in 2024 or
later.
So I've replaced the `#ifdef` in `llvm-libc-types/time_t.h` with two
versions of the header file, chosen in `CMakeLists.txt` via a new
configuration option. This involved adding an extra parameter to the
cmake `add_header` function to specify different names for the header
file in the source and destination directories.
---
libc/cmake/modules/LLVMLibCHeaderRules.cmake | 8 ++++++--
libc/config/config.json | 6 ++++++
libc/docs/configure.rst | 2 ++
libc/include/llvm-libc-types/CMakeLists.txt | 11 ++++++++++-
libc/include/llvm-libc-types/time_t_32.h | 14 ++++++++++++++
.../llvm-libc-types/{time_t.h => time_t_64.h} | 4 ----
6 files changed, 38 insertions(+), 7 deletions(-)
create mode 100644 libc/include/llvm-libc-types/time_t_32.h
rename libc/include/llvm-libc-types/{time_t.h => time_t_64.h} (85%)
diff --git a/libc/cmake/modules/LLVMLibCHeaderRules.cmake b/libc/cmake/modules/LLVMLibCHeaderRules.cmake
index 3049f4db7301f6..5b29fa7897e1c2 100644
--- a/libc/cmake/modules/LLVMLibCHeaderRules.cmake
+++ b/libc/cmake/modules/LLVMLibCHeaderRules.cmake
@@ -10,7 +10,7 @@ function(add_header target_name)
cmake_parse_arguments(
"ADD_HEADER"
"" # No optional arguments
- "HDR" # Single value arguments
+ "HDR;SRCHDR" # Single value arguments
"DEPENDS"
${ARGN}
)
@@ -21,7 +21,11 @@ function(add_header target_name)
set(absolute_path ${CMAKE_CURRENT_SOURCE_DIR}/${ADD_HEADER_HDR})
file(RELATIVE_PATH relative_path ${LIBC_INCLUDE_SOURCE_DIR} ${absolute_path})
set(dest_file ${LIBC_INCLUDE_DIR}/${relative_path})
- set(src_file ${CMAKE_CURRENT_SOURCE_DIR}/${ADD_HEADER_HDR})
+ if(ADD_HEADER_SRCHDR)
+ set(src_file ${CMAKE_CURRENT_SOURCE_DIR}/${ADD_HEADER_SRCHDR})
+ else()
+ set(src_file ${CMAKE_CURRENT_SOURCE_DIR}/${ADD_HEADER_HDR})
+ endif()
add_custom_command(
OUTPUT ${dest_file}
diff --git a/libc/config/config.json b/libc/config/config.json
index 538fea53cc704a..2e72c0a3fd1d69 100644
--- a/libc/config/config.json
+++ b/libc/config/config.json
@@ -88,5 +88,11 @@
"value": true,
"doc": "Make setjmp save the value of x18, and longjmp restore it. The AArch64 ABI delegates this register to platform ABIs, which can choose whether to make it caller-saved."
}
+ },
+ "time": {
+ "LIBC_CONF_TIME_64BIT": {
+ "value": false,
+ "doc": "Force the size of time_t to 64 bits, even on platforms where compatibility considerations would otherwise make it 32-bit."
+ }
}
}
diff --git a/libc/docs/configure.rst b/libc/docs/configure.rst
index 950de0eee4c05d..54ca5d55d7b243 100644
--- a/libc/docs/configure.rst
+++ b/libc/docs/configure.rst
@@ -52,3 +52,5 @@ to learn about the defaults for your platform and target.
* **"string" options**
- ``LIBC_CONF_MEMSET_X86_USE_SOFTWARE_PREFETCHING``: Inserts prefetch for write instructions (PREFETCHW) for memset on x86 to recover performance when hardware prefetcher is disabled.
- ``LIBC_CONF_STRING_UNSAFE_WIDE_READ``: Read more than a byte at a time to perform byte-string operations like strlen.
+* **"time" options**
+ - ``LIBC_CONF_TIME_64BIT``: Force the size of time_t to 64 bits, even on platforms where compatibility considerations would otherwise make it 32-bit.
diff --git a/libc/include/llvm-libc-types/CMakeLists.txt b/libc/include/llvm-libc-types/CMakeLists.txt
index d8b975572e0dd3..3dfa39b9b132b0 100644
--- a/libc/include/llvm-libc-types/CMakeLists.txt
+++ b/libc/include/llvm-libc-types/CMakeLists.txt
@@ -58,7 +58,16 @@ add_header(pthread_rwlock_t HDR pthread_rwlock_t.h DEPENDS .__futex_word .pid_t)
add_header(pthread_rwlockattr_t HDR pthread_rwlockattr_t.h)
add_header(pthread_t HDR pthread_t.h DEPENDS .__thread_type)
add_header(rlim_t HDR rlim_t.h)
-add_header(time_t HDR time_t.h)
+if(LIBC_CONF_TIME_64BIT)
+ # Set time_t to 64 bit as requested in configuration
+ add_header(time_t HDR time_t.h SRCHDR time_t_64.h)
+elseif(LIBC_TARGET_ARCHITECTURE_IS_ARM)
+ # Set time_t to 32 bit for compatibility with glibc
+ add_header(time_t HDR time_t.h SRCHDR time_t_32.h)
+else()
+ # Other platforms default to 64-bit time_t
+ add_header(time_t HDR time_t.h SRCHDR time_t_64.h)
+endif()
add_header(stack_t HDR stack_t.h DEPENDS .size_t)
add_header(suseconds_t HDR suseconds_t.h)
add_header(struct_flock HDR struct_flock.h DEPENDS .off_t .pid_t)
diff --git a/libc/include/llvm-libc-types/time_t_32.h b/libc/include/llvm-libc-types/time_t_32.h
new file mode 100644
index 00000000000000..ce5a3e8b734636
--- /dev/null
+++ b/libc/include/llvm-libc-types/time_t_32.h
@@ -0,0 +1,14 @@
+//===-- Definition of the type time_t -------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_TYPES_TIME_T_H
+#define LLVM_LIBC_TYPES_TIME_T_H
+
+typedef __INT32_TYPE__ time_t;
+
+#endif // LLVM_LIBC_TYPES_TIME_T_H
diff --git a/libc/include/llvm-libc-types/time_t.h b/libc/include/llvm-libc-types/time_t_64.h
similarity index 85%
rename from libc/include/llvm-libc-types/time_t.h
rename to libc/include/llvm-libc-types/time_t_64.h
index 59953b343ba963..c2c909e226144d 100644
--- a/libc/include/llvm-libc-types/time_t.h
+++ b/libc/include/llvm-libc-types/time_t_64.h
@@ -9,10 +9,6 @@
#ifndef LLVM_LIBC_TYPES_TIME_T_H
#define LLVM_LIBC_TYPES_TIME_T_H
-#if (defined(__arm__) || defined(_M_ARM))
-typedef __INTPTR_TYPE__ time_t;
-#else
typedef __INT64_TYPE__ time_t;
-#endif
#endif // LLVM_LIBC_TYPES_TIME_T_H
>From 795146ff04bc3fd62627d49c0e9decb061ba7fc3 Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tatham at arm.com>
Date: Tue, 6 Aug 2024 16:42:33 +0100
Subject: [PATCH 2/3] Review feedback: rework how the right time_t header is
selected.
Now the decision results in LIBC_TYPES_TIME_T_IS_32_BIT being set in
the cmake scripts. That controls which of time_t_{32,64}.h is copied
to the output header time_t.h. But it also sets a flag on the compile
command lines, which cause hdr/llvm-libc-types/time_t.h to #include
the same one of those headers at library build time.
---
.../modules/LLVMLibCCompileOptionRules.cmake | 4 ++++
libc/cmake/modules/LLVMLibCFlagRules.cmake | 13 +++++++++++++
libc/cmake/modules/LLVMLibCHeaderRules.cmake | 17 +++++++++--------
libc/include/llvm-libc-types/CMakeLists.txt | 11 +++--------
libc/include/llvm-libc-types/time_t.h | 18 ++++++++++++++++++
5 files changed, 47 insertions(+), 16 deletions(-)
create mode 100644 libc/include/llvm-libc-types/time_t.h
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index 9fc10375a1d379..278a61a2d3cd51 100644
--- a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -71,6 +71,10 @@ function(_get_compile_options_from_config output_var)
list(APPEND config_options "-DLIBC_QSORT_IMPL=${LIBC_CONF_QSORT_IMPL}")
endif()
+ if(LIBC_TYPES_TIME_T_IS_32_BIT)
+ list(APPEND config_options "-DLIBC_TYPES_TIME_T_IS_32_BIT")
+ endif()
+
set(${output_var} ${config_options} PARENT_SCOPE)
endfunction(_get_compile_options_from_config)
diff --git a/libc/cmake/modules/LLVMLibCFlagRules.cmake b/libc/cmake/modules/LLVMLibCFlagRules.cmake
index 3629a7f111a7c5..69f31ace80dd3d 100644
--- a/libc/cmake/modules/LLVMLibCFlagRules.cmake
+++ b/libc/cmake/modules/LLVMLibCFlagRules.cmake
@@ -286,3 +286,16 @@ if(NOT((LIBC_TARGET_ARCHITECTURE_IS_X86 AND (LIBC_CPU_FEATURES MATCHES "SSE4_2")
LIBC_TARGET_ARCHITECTURE_IS_AARCH64 OR LIBC_TARGET_OS_IS_GPU))
set(SKIP_FLAG_EXPANSION_ROUND_OPT TRUE)
endif()
+
+# Choose whether time_t is 32- or 64-bit, based on target architecture
+# and config options. This will be used to set a #define during the
+# library build, and also to select the right version of time_t.h for
+# the output headers.
+if(LIBC_TARGET_ARCHITECTURE_IS_ARM AND NOT (LIBC_CONF_TIME_64BIT))
+ # Set time_t to 32 bit for compatibility with glibc, unless
+ # configuration says otherwise
+ set(LIBC_TYPES_TIME_T_IS_32_BIT TRUE)
+else()
+ # Other platforms default to 64-bit time_t
+ set(LIBC_TYPES_TIME_T_IS_32_BIT FALSE)
+endif()
diff --git a/libc/cmake/modules/LLVMLibCHeaderRules.cmake b/libc/cmake/modules/LLVMLibCHeaderRules.cmake
index 5b29fa7897e1c2..c2c675bda26d31 100644
--- a/libc/cmake/modules/LLVMLibCHeaderRules.cmake
+++ b/libc/cmake/modules/LLVMLibCHeaderRules.cmake
@@ -9,8 +9,8 @@
function(add_header target_name)
cmake_parse_arguments(
"ADD_HEADER"
- "" # No optional arguments
- "HDR;SRCHDR" # Single value arguments
+ "" # No optional arguments
+ "HDR;DEST_HDR" # Single value arguments
"DEPENDS"
${ARGN}
)
@@ -18,14 +18,15 @@ function(add_header target_name)
message(FATAL_ERROR "'add_header' rules requires the HDR argument specifying a headef file.")
endif()
- set(absolute_path ${CMAKE_CURRENT_SOURCE_DIR}/${ADD_HEADER_HDR})
- file(RELATIVE_PATH relative_path ${LIBC_INCLUDE_SOURCE_DIR} ${absolute_path})
- set(dest_file ${LIBC_INCLUDE_DIR}/${relative_path})
- if(ADD_HEADER_SRCHDR)
- set(src_file ${CMAKE_CURRENT_SOURCE_DIR}/${ADD_HEADER_SRCHDR})
+ if(ADD_HEADER_DEST_HDR)
+ set(dest_leaf_filename ${ADD_HEADER_DEST_HDR})
else()
- set(src_file ${CMAKE_CURRENT_SOURCE_DIR}/${ADD_HEADER_HDR})
+ set(dest_leaf_filename ${ADD_HEADER_HDR})
endif()
+ set(absolute_path ${CMAKE_CURRENT_SOURCE_DIR}/${dest_leaf_filename})
+ file(RELATIVE_PATH relative_path ${LIBC_INCLUDE_SOURCE_DIR} ${absolute_path})
+ set(dest_file ${LIBC_INCLUDE_DIR}/${relative_path})
+ set(src_file ${CMAKE_CURRENT_SOURCE_DIR}/${ADD_HEADER_HDR})
add_custom_command(
OUTPUT ${dest_file}
diff --git a/libc/include/llvm-libc-types/CMakeLists.txt b/libc/include/llvm-libc-types/CMakeLists.txt
index 3dfa39b9b132b0..196abc80a2322f 100644
--- a/libc/include/llvm-libc-types/CMakeLists.txt
+++ b/libc/include/llvm-libc-types/CMakeLists.txt
@@ -58,15 +58,10 @@ add_header(pthread_rwlock_t HDR pthread_rwlock_t.h DEPENDS .__futex_word .pid_t)
add_header(pthread_rwlockattr_t HDR pthread_rwlockattr_t.h)
add_header(pthread_t HDR pthread_t.h DEPENDS .__thread_type)
add_header(rlim_t HDR rlim_t.h)
-if(LIBC_CONF_TIME_64BIT)
- # Set time_t to 64 bit as requested in configuration
- add_header(time_t HDR time_t.h SRCHDR time_t_64.h)
-elseif(LIBC_TARGET_ARCHITECTURE_IS_ARM)
- # Set time_t to 32 bit for compatibility with glibc
- add_header(time_t HDR time_t.h SRCHDR time_t_32.h)
+if(LIBC_TYPES_TIME_T_IS_32_BIT)
+ add_header(time_t HDR time_t_32.h DEST_HDR time_t.h)
else()
- # Other platforms default to 64-bit time_t
- add_header(time_t HDR time_t.h SRCHDR time_t_64.h)
+ add_header(time_t HDR time_t_64.h DEST_HDR time_t.h)
endif()
add_header(stack_t HDR stack_t.h DEPENDS .size_t)
add_header(suseconds_t HDR suseconds_t.h)
diff --git a/libc/include/llvm-libc-types/time_t.h b/libc/include/llvm-libc-types/time_t.h
new file mode 100644
index 00000000000000..76920dc07ec69c
--- /dev/null
+++ b/libc/include/llvm-libc-types/time_t.h
@@ -0,0 +1,18 @@
+//===-- Definition of the type time_t, for use during the libc build ------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_TYPES_TIME_T_H
+#define LLVM_LIBC_TYPES_TIME_T_H
+
+#ifdef LIBC_TYPES_TIME_T_IS_32_BIT
+#include "time_t_32.h"
+#else
+#include "time_t_64.h"
+#endif
+
+#endif // LLVM_LIBC_TYPES_TIME_T_H
>From 513115835cee2499a5926f33549937fb10790af0 Mon Sep 17 00:00:00 2001
From: Simon Tatham <simon.tatham at arm.com>
Date: Thu, 8 Aug 2024 16:08:31 +0100
Subject: [PATCH 3/3] Review nits: disambiguate guard macros, conditionalise -D
---
libc/cmake/modules/LLVMLibCCompileOptionRules.cmake | 2 +-
libc/include/llvm-libc-types/time_t_32.h | 6 +++---
libc/include/llvm-libc-types/time_t_64.h | 6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index 278a61a2d3cd51..e3dfe1a1529691 100644
--- a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -71,7 +71,7 @@ function(_get_compile_options_from_config output_var)
list(APPEND config_options "-DLIBC_QSORT_IMPL=${LIBC_CONF_QSORT_IMPL}")
endif()
- if(LIBC_TYPES_TIME_T_IS_32_BIT)
+ if(LIBC_TYPES_TIME_T_IS_32_BIT AND LLVM_LIBC_FULL_BUILD)
list(APPEND config_options "-DLIBC_TYPES_TIME_T_IS_32_BIT")
endif()
diff --git a/libc/include/llvm-libc-types/time_t_32.h b/libc/include/llvm-libc-types/time_t_32.h
index ce5a3e8b734636..2c415f6fa9dcab 100644
--- a/libc/include/llvm-libc-types/time_t_32.h
+++ b/libc/include/llvm-libc-types/time_t_32.h
@@ -6,9 +6,9 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_LIBC_TYPES_TIME_T_H
-#define LLVM_LIBC_TYPES_TIME_T_H
+#ifndef LLVM_LIBC_TYPES_TIME_T_32_H
+#define LLVM_LIBC_TYPES_TIME_T_32_H
typedef __INT32_TYPE__ time_t;
-#endif // LLVM_LIBC_TYPES_TIME_T_H
+#endif // LLVM_LIBC_TYPES_TIME_T_32_H
diff --git a/libc/include/llvm-libc-types/time_t_64.h b/libc/include/llvm-libc-types/time_t_64.h
index c2c909e226144d..8f7fd3233646e6 100644
--- a/libc/include/llvm-libc-types/time_t_64.h
+++ b/libc/include/llvm-libc-types/time_t_64.h
@@ -6,9 +6,9 @@
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_LIBC_TYPES_TIME_T_H
-#define LLVM_LIBC_TYPES_TIME_T_H
+#ifndef LLVM_LIBC_TYPES_TIME_T_64_H
+#define LLVM_LIBC_TYPES_TIME_T_64_H
typedef __INT64_TYPE__ time_t;
-#endif // LLVM_LIBC_TYPES_TIME_T_H
+#endif // LLVM_LIBC_TYPES_TIME_T_64_H
More information about the libc-commits
mailing list