[libc-commits] [libc] de939c6 - [libc] enable printf using system FILE
Michael Jones via libc-commits
libc-commits at lists.llvm.org
Thu Mar 23 09:56:51 PDT 2023
Author: Michael Jones
Date: 2023-03-23T09:56:45-07:00
New Revision: de939c6cd80c1e88913f1ef12be11aea501eb89b
URL: https://github.com/llvm/llvm-project/commit/de939c6cd80c1e88913f1ef12be11aea501eb89b
DIFF: https://github.com/llvm/llvm-project/commit/de939c6cd80c1e88913f1ef12be11aea501eb89b.diff
LOG: [libc] enable printf using system FILE
The printf and fprintf implementations use our internal implementation
to improve performance when it's available, but this patch enables using
the public FILE API for overlay mode.
Reviewed By: sivachandra, lntue
Differential Revision: https://reviews.llvm.org/D146001
Added:
Modified:
libc/config/linux/x86_64/entrypoints.txt
libc/src/stdio/CMakeLists.txt
libc/src/stdio/fprintf.cpp
libc/src/stdio/printf.cpp
libc/src/stdio/printf_core/CMakeLists.txt
libc/src/stdio/printf_core/file_writer.h
libc/src/stdio/printf_core/vfprintf_internal.h
libc/test/src/stdio/CMakeLists.txt
libc/test/src/stdio/fprintf_test.cpp
Removed:
libc/src/stdio/printf_core/file_writer.cpp
libc/src/stdio/printf_core/vfprintf_internal.cpp
################################################################################
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index b3017338f8260..5c0b3103f5615 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -111,6 +111,8 @@ set(TARGET_LIBC_ENTRYPOINTS
libc.src.stdio.remove
libc.src.stdio.sprintf
libc.src.stdio.snprintf
+ libc.src.stdio.fprintf
+ libc.src.stdio.printf
# sys/mman.h entrypoints
libc.src.sys.mman.madvise
@@ -412,10 +414,8 @@ if(LLVM_LIBC_FULL_BUILD)
libc.src.stdio.funlockfile
libc.src.stdio.fwrite
libc.src.stdio.fwrite_unlocked
- libc.src.stdio.fprintf
libc.src.stdio.getc
libc.src.stdio.getc_unlocked
- libc.src.stdio.printf
libc.src.stdio.sscanf
libc.src.stdio.scanf
libc.src.stdio.fscanf
diff --git a/libc/src/stdio/CMakeLists.txt b/libc/src/stdio/CMakeLists.txt
index 5f8d17953f633..7ccbf9aa28c4c 100644
--- a/libc/src/stdio/CMakeLists.txt
+++ b/libc/src/stdio/CMakeLists.txt
@@ -480,29 +480,42 @@ add_entrypoint_object(
libc.src.stdio.printf_core.writer
)
+list(APPEND printf_deps
+ libc.src.__support.arg_list
+ libc.src.stdio.printf_core.vfprintf_internal
+)
+if(LLVM_LIBC_FULL_BUILD)
+ list(APPEND printf_deps
+ libc.src.__support.File.file
+ libc.src.__support.File.platform_file
+ )
+else()
+ set(printf_copts "-DLIBC_COPT_PRINTF_USE_SYSTEM_FILE")
+endif()
+
add_entrypoint_object(
- fprintf
+ printf
SRCS
- fprintf.cpp
+ printf.cpp
HDRS
- fprintf.h
+ printf.h
DEPENDS
- libc.src.__support.arg_list
- libc.src.stdio.printf_core.vfprintf_internal
+ ${printf_deps}
+ COMPILE_OPTIONS
+ ${printf_copts}
)
-
add_entrypoint_object(
- printf
+ fprintf
SRCS
- printf.cpp
+ fprintf.cpp
HDRS
- printf.h
+ fprintf.h
DEPENDS
- libc.src.__support.File.file
- libc.src.__support.File.platform_file
libc.src.__support.arg_list
libc.src.stdio.printf_core.vfprintf_internal
+ COMPILE_OPTIONS
+ ${printf_copts}
)
add_entrypoint_object(
diff --git a/libc/src/stdio/fprintf.cpp b/libc/src/stdio/fprintf.cpp
index 796d5b5c47095..da8fabf5ab542 100644
--- a/libc/src/stdio/fprintf.cpp
+++ b/libc/src/stdio/fprintf.cpp
@@ -13,9 +13,16 @@
#include "src/stdio/printf_core/vfprintf_internal.h"
#include <stdarg.h>
+#include <stdio.h>
namespace __llvm_libc {
+#ifndef LIBC_COPT_PRINTF_USE_SYSTEM_FILE
+using FileT = __llvm_libc::File;
+#else // defined(LIBC_COPT_PRINTF_USE_SYSTEM_FILE)
+using FileT = ::FILE;
+#endif // LIBC_COPT_PRINTF_USE_SYSTEM_FILE
+
LLVM_LIBC_FUNCTION(int, fprintf,
(::FILE *__restrict stream, const char *__restrict format,
...)) {
@@ -25,7 +32,8 @@ LLVM_LIBC_FUNCTION(int, fprintf,
// and pointer semantics, as well as handling
// destruction automatically.
va_end(vlist);
- int ret_val = printf_core::vfprintf_internal(stream, format, args);
+ int ret_val = printf_core::vfprintf_internal(
+ reinterpret_cast<FileT *>(stream), format, args);
return ret_val;
}
diff --git a/libc/src/stdio/printf.cpp b/libc/src/stdio/printf.cpp
index 8fd8b9cc57fad..ca6f61ed63033 100644
--- a/libc/src/stdio/printf.cpp
+++ b/libc/src/stdio/printf.cpp
@@ -8,11 +8,18 @@
#include "src/stdio/printf.h"
-#include "src/__support/File/file.h"
#include "src/__support/arg_list.h"
#include "src/stdio/printf_core/vfprintf_internal.h"
#include <stdarg.h>
+#include <stdio.h>
+
+#ifndef LIBC_COPT_PRINTF_USE_SYSTEM_FILE
+#include "src/__support/File/file.h"
+#define PRINTF_STDOUT __llvm_libc::stdout
+#else // LIBC_COPT_PRINTF_USE_SYSTEM_FILE
+#define PRINTF_STDOUT ::stdout
+#endif // LIBC_COPT_PRINTF_USE_SYSTEM_FILE
namespace __llvm_libc {
@@ -23,8 +30,7 @@ LLVM_LIBC_FUNCTION(int, printf, (const char *__restrict format, ...)) {
// and pointer semantics, as well as handling
// destruction automatically.
va_end(vlist);
- int ret_val = printf_core::vfprintf_internal(
- reinterpret_cast<::FILE *>(__llvm_libc::stdout), format, args);
+ int ret_val = printf_core::vfprintf_internal(PRINTF_STDOUT, format, args);
return ret_val;
}
diff --git a/libc/src/stdio/printf_core/CMakeLists.txt b/libc/src/stdio/printf_core/CMakeLists.txt
index 31db8ad3c524c..109399772b53d 100644
--- a/libc/src/stdio/printf_core/CMakeLists.txt
+++ b/libc/src/stdio/printf_core/CMakeLists.txt
@@ -116,35 +116,31 @@ add_object_library(
libc.src.__support.arg_list
)
-if(NOT (TARGET libc.src.__support.File.file))
- # Not all platforms have a file implementation. If file is unvailable,
- # then we must skip all file based printf sections.
+if(NOT (TARGET libc.src.__support.File.file) AND LLVM_LIBC_FULL_BUILD)
+ # Not all platforms have a file implementation. If file is unvailable, and a
+ # full build is requested, then we must skip all file based printf sections.
return()
endif()
-add_object_library(
+add_header_library(
file_writer
- SRCS
- file_writer.cpp
HDRS
file_writer.h
DEPENDS
+ libc.include.stdio
libc.src.__support.File.file
libc.src.__support.CPP.string_view
libc.src.string.memory_utils.memset_implementation
.core_structs
)
-add_object_library(
+add_header_library(
vfprintf_internal
- SRCS
- vfprintf_internal.cpp
HDRS
vfprintf_internal.h
DEPENDS
libc.include.stdio
libc.src.__support.File.file
- libc.src.__support.File.platform_file
libc.src.__support.arg_list
libc.src.stdio.printf_core.printf_main
libc.src.stdio.printf_core.file_writer
diff --git a/libc/src/stdio/printf_core/file_writer.cpp b/libc/src/stdio/printf_core/file_writer.cpp
deleted file mode 100644
index 0e07e1c1eb8a7..0000000000000
--- a/libc/src/stdio/printf_core/file_writer.cpp
+++ /dev/null
@@ -1,54 +0,0 @@
-//===-- FILE Writer implementation for printf -------------------*- C++ -*-===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-#include "src/stdio/printf_core/file_writer.h"
-#include "src/__support/CPP/string_view.h"
-#include "src/__support/File/file.h"
-#include "src/stdio/printf_core/core_structs.h"
-#include <stddef.h>
-
-namespace __llvm_libc {
-namespace printf_core {
-
-int FileWriter::write(const char *__restrict to_write, size_t len) {
- auto result = file->write_unlocked(to_write, len);
- int written = result.value;
- if (written != static_cast<int>(len) || result.has_error())
- written = FILE_WRITE_ERROR;
- if (file->error_unlocked())
- written = FILE_STATUS_ERROR;
- return written;
-}
-
-int FileWriter::write_str(void *raw_pointer, cpp::string_view new_string) {
- FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
- return file_writer->write(new_string.data(), new_string.size());
-}
-
-int FileWriter::write_chars(void *raw_pointer, char new_char, size_t len) {
- FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
- constexpr size_t BUFF_SIZE = 8;
- char buff[BUFF_SIZE] = {new_char};
- int result;
- while (len > BUFF_SIZE) {
- result = file_writer->write(buff, BUFF_SIZE);
- if (result < 0)
- return result;
- len -= BUFF_SIZE;
- }
- return file_writer->write(buff, len);
-}
-
-// TODO(michaelrj): Move this to putc_unlocked once that is available.
-int FileWriter::write_char(void *raw_pointer, char new_char) {
- FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
- return file_writer->write(&new_char, 1);
-}
-
-} // namespace printf_core
-} // namespace __llvm_libc
diff --git a/libc/src/stdio/printf_core/file_writer.h b/libc/src/stdio/printf_core/file_writer.h
index 6ba1428a160e2..0fd6d115ddd8b 100644
--- a/libc/src/stdio/printf_core/file_writer.h
+++ b/libc/src/stdio/printf_core/file_writer.h
@@ -11,6 +11,8 @@
#include "src/__support/CPP/string_view.h"
#include "src/__support/File/file.h"
+#include "src/__support/macros/attributes.h" // For LIBC_INLINE
+#include "src/stdio/printf_core/core_structs.h"
#include <stddef.h>
#include <stdio.h>
@@ -18,26 +20,81 @@
namespace __llvm_libc {
namespace printf_core {
-class FileWriter {
- __llvm_libc::File *file;
+template <typename file_t> class FileWriter {
+ file_t *file;
public:
- FileWriter(::FILE *init_file) {
- file = reinterpret_cast<__llvm_libc::File *>(init_file);
- file->lock();
- }
+ LIBC_INLINE FileWriter(file_t *init_file);
- ~FileWriter() { file->unlock(); }
+ LIBC_INLINE ~FileWriter();
- int write(const char *__restrict to_write, size_t len);
+ LIBC_INLINE int write(const char *__restrict to_write, size_t len);
// These write functions take a FileWriter as a void* in raw_pointer, and
// call the appropriate write function on it.
- static int write_str(void *raw_pointer, cpp::string_view new_string);
- static int write_chars(void *raw_pointer, char new_char, size_t len);
- static int write_char(void *raw_pointer, char new_char);
+ static int write_str(void *raw_pointer, cpp::string_view new_string) {
+ FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
+ return file_writer->write(new_string.data(), new_string.size());
+ }
+ static int write_chars(void *raw_pointer, char new_char, size_t len) {
+ FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
+ constexpr size_t BUFF_SIZE = 8;
+ char buff[BUFF_SIZE] = {new_char};
+ int result;
+ while (len > BUFF_SIZE) {
+ result = file_writer->write(buff, BUFF_SIZE);
+ if (result < 0)
+ return result;
+ len -= BUFF_SIZE;
+ }
+ return file_writer->write(buff, len);
+ }
+ static int write_char(void *raw_pointer, char new_char) {
+ FileWriter *file_writer = reinterpret_cast<FileWriter *>(raw_pointer);
+ return file_writer->write(&new_char, 1);
+ }
};
+// The interface for using our internal file implementation.
+template <>
+LIBC_INLINE
+FileWriter<__llvm_libc::File>::FileWriter(__llvm_libc::File *init_file) {
+ file = init_file;
+ file->lock();
+}
+template <> LIBC_INLINE FileWriter<__llvm_libc::File>::~FileWriter() {
+ file->unlock();
+}
+template <>
+LIBC_INLINE int
+FileWriter<__llvm_libc::File>::write(const char *__restrict to_write,
+ size_t len) {
+ auto result = file->write_unlocked(to_write, len);
+ size_t written = result.value;
+ if (written != len || result.has_error())
+ written = FILE_WRITE_ERROR;
+ if (file->error_unlocked())
+ written = FILE_STATUS_ERROR;
+ return written;
+}
+
+// The interface for using the system's file implementation.
+template <> LIBC_INLINE FileWriter<::FILE>::FileWriter(::FILE *init_file) {
+ file = init_file;
+ ::flockfile(file);
+}
+template <> LIBC_INLINE FileWriter<::FILE>::~FileWriter() {
+ ::funlockfile(file);
+}
+template <>
+LIBC_INLINE int FileWriter<::FILE>::write(const char *__restrict to_write,
+ size_t len) {
+ size_t written = ::fwrite_unlocked(to_write, 1, len, file);
+ if (written != len || ::ferror_unlocked(file))
+ written = FILE_WRITE_ERROR;
+ return written;
+}
+
} // namespace printf_core
} // namespace __llvm_libc
diff --git a/libc/src/stdio/printf_core/vfprintf_internal.cpp b/libc/src/stdio/printf_core/vfprintf_internal.cpp
deleted file mode 100644
index b25d545e54a11..0000000000000
--- a/libc/src/stdio/printf_core/vfprintf_internal.cpp
+++ /dev/null
@@ -1,32 +0,0 @@
-//===-- Internal implementation of vfprintf ---------------------*- C++ -*-===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-#include "src/stdio/printf_core/vfprintf_internal.h"
-
-#include "src/__support/arg_list.h"
-#include "src/stdio/printf_core/file_writer.h"
-#include "src/stdio/printf_core/printf_main.h"
-#include "src/stdio/printf_core/writer.h"
-
-#include <stdio.h>
-
-namespace __llvm_libc {
-namespace printf_core {
-
-int vfprintf_internal(::FILE *__restrict stream, const char *__restrict format,
- internal::ArgList &args) {
- FileWriter file_writer(stream);
- printf_core::Writer writer(reinterpret_cast<void *>(&file_writer),
- printf_core::FileWriter::write_str,
- printf_core::FileWriter::write_chars,
- printf_core::FileWriter::write_char);
- return printf_core::printf_main(&writer, format, args);
-}
-
-} // namespace printf_core
-} // namespace __llvm_libc
diff --git a/libc/src/stdio/printf_core/vfprintf_internal.h b/libc/src/stdio/printf_core/vfprintf_internal.h
index b837ebba182b4..762018f0b04c4 100644
--- a/libc/src/stdio/printf_core/vfprintf_internal.h
+++ b/libc/src/stdio/printf_core/vfprintf_internal.h
@@ -9,15 +9,29 @@
#ifndef LLVM_LIBC_SRC_STDIO_PRINTF_CORE_VFPRINTF_INTERNAL_H
#define LLVM_LIBC_SRC_STDIO_PRINTF_CORE_VFPRINTF_INTERNAL_H
+#include "src/__support/File/file.h"
#include "src/__support/arg_list.h"
+#include "src/__support/macros/attributes.h" // For LIBC_INLINE
+#include "src/stdio/printf_core/file_writer.h"
+#include "src/stdio/printf_core/printf_main.h"
+#include "src/stdio/printf_core/writer.h"
#include <stdio.h>
namespace __llvm_libc {
namespace printf_core {
-int vfprintf_internal(::FILE *__restrict stream, const char *__restrict format,
- internal::ArgList &args);
+template <typename file_t>
+LIBC_INLINE int vfprintf_internal(file_t *__restrict stream,
+ const char *__restrict format,
+ internal::ArgList &args) {
+ FileWriter<file_t> file_writer(stream);
+ Writer writer(reinterpret_cast<void *>(&file_writer),
+ FileWriter<file_t>::write_str, FileWriter<file_t>::write_chars,
+ FileWriter<file_t>::write_char);
+ return printf_main(&writer, format, args);
+}
+
} // namespace printf_core
} // namespace __llvm_libc
diff --git a/libc/test/src/stdio/CMakeLists.txt b/libc/test/src/stdio/CMakeLists.txt
index 8747f18f9045b..a4b5a9be892f1 100644
--- a/libc/test/src/stdio/CMakeLists.txt
+++ b/libc/test/src/stdio/CMakeLists.txt
@@ -134,6 +134,8 @@ add_libc_unittest(
libc.src.stdio.snprintf
)
+# In fullbuild mode, fprintf's tests use the internal FILE for other functions.
+if(LLVM_LIBC_FULL_BUILD)
add_libc_unittest(
fprintf_test
SUITE
@@ -147,7 +149,20 @@ add_libc_unittest(
libc.src.stdio.fopen
libc.src.stdio.fread
)
-
+else()
+# Else in overlay mode they use the system's FILE.
+add_libc_unittest(
+ fprintf_test
+ SUITE
+ libc_stdio_unittests
+ SRCS
+ fprintf_test.cpp
+ DEPENDS
+ libc.src.stdio.fprintf
+ COMPILE_OPTIONS
+ -DLIBC_COPT_PRINTF_USE_SYSTEM_FILE
+)
+endif()
add_libc_unittest(
printf_test
diff --git a/libc/test/src/stdio/fprintf_test.cpp b/libc/test/src/stdio/fprintf_test.cpp
index 286c516fbcf96..20b3c0faed6f7 100644
--- a/libc/test/src/stdio/fprintf_test.cpp
+++ b/libc/test/src/stdio/fprintf_test.cpp
@@ -6,10 +6,12 @@
//
//===----------------------------------------------------------------------===//
+#ifndef LIBC_COPT_PRINTF_USE_SYSTEM_FILE
#include "src/stdio/fclose.h"
#include "src/stdio/ferror.h"
#include "src/stdio/fopen.h"
#include "src/stdio/fread.h"
+#endif // LIBC_COPT_PRINTF_USE_SYSTEM_FILE
#include "src/stdio/fprintf.h"
@@ -17,9 +19,23 @@
#include <stdio.h>
+namespace printf_test {
+#ifndef LIBC_COPT_PRINTF_USE_SYSTEM_FILE
+using __llvm_libc::fclose;
+using __llvm_libc::ferror;
+using __llvm_libc::fopen;
+using __llvm_libc::fread;
+#else // defined(LIBC_COPT_PRINTF_USE_SYSTEM_FILE)
+using ::fclose;
+using ::ferror;
+using ::fopen;
+using ::fread;
+#endif // LIBC_COPT_PRINTF_USE_SYSTEM_FILE
+} // namespace printf_test
+
TEST(LlvmLibcFPrintfTest, WriteToFile) {
constexpr char FILENAME[] = "testdata/fprintf_output.test";
- ::FILE *file = __llvm_libc::fopen(FILENAME, "w");
+ ::FILE *file = printf_test::fopen(FILENAME, "w");
ASSERT_FALSE(file == nullptr);
int written;
@@ -37,31 +53,31 @@ TEST(LlvmLibcFPrintfTest, WriteToFile) {
written = __llvm_libc::fprintf(file, format_more, short_numbers);
EXPECT_EQ(written, 14);
- ASSERT_EQ(0, __llvm_libc::fclose(file));
+ ASSERT_EQ(0, printf_test::fclose(file));
- file = __llvm_libc::fopen(FILENAME, "r");
+ file = printf_test::fopen(FILENAME, "r");
ASSERT_FALSE(file == nullptr);
char data[50];
- ASSERT_EQ(__llvm_libc::fread(data, 1, sizeof(simple) - 1, file),
+ ASSERT_EQ(printf_test::fread(data, 1, sizeof(simple) - 1, file),
sizeof(simple) - 1);
data[sizeof(simple) - 1] = '\0';
ASSERT_STREQ(data, simple);
- ASSERT_EQ(__llvm_libc::fread(data, 1, sizeof(numbers) - 1, file),
+ ASSERT_EQ(printf_test::fread(data, 1, sizeof(numbers) - 1, file),
sizeof(numbers) - 1);
data[sizeof(numbers) - 1] = '\0';
ASSERT_STREQ(data, numbers);
- ASSERT_EQ(__llvm_libc::fread(
+ ASSERT_EQ(printf_test::fread(
data, 1, sizeof(format_more) + sizeof(short_numbers) - 4, file),
sizeof(format_more) + sizeof(short_numbers) - 4);
data[sizeof(format_more) + sizeof(short_numbers) - 4] = '\0';
ASSERT_STREQ(data, "1234 and more\n");
- ASSERT_EQ(__llvm_libc::ferror(file), 0);
+ ASSERT_EQ(printf_test::ferror(file), 0);
written =
__llvm_libc::fprintf(file, "Writing to a read only file should fail.");
EXPECT_LT(written, 0);
- ASSERT_EQ(__llvm_libc::fclose(file), 0);
+ ASSERT_EQ(printf_test::fclose(file), 0);
}
More information about the libc-commits
mailing list