[libc-commits] [libc] 9a99afb - [libc] Use string_view for write_to_stderr

[Guillaume Chatelet via libc-commits]

Author: Guillaume Chatelet
Date: 2023-04-02T14:52:33Z
New Revision: 9a99afb455601aaa3a28d307c7bbcfdaad3fa4fc

URL: https://github.com/llvm/llvm-project/commit/9a99afb455601aaa3a28d307c7bbcfdaad3fa4fc
DIFF: https://github.com/llvm/llvm-project/commit/9a99afb455601aaa3a28d307c7bbcfdaad3fa4fc.diff

LOG: [libc] Use string_view for write_to_stderr

This patch makes use of `cpp::string_view` instead of `const char*` for `write_to_stderr`. This helps sending non null-terminated buffers such as a single character, `cpp::string_view` or `cpp::string`.
It also fizes the gpu version that had several bugs (See https://reviews.llvm.org/D145913#4236641).

Differential Revision: https://reviews.llvm.org/D147375

Added: 
    

Modified: 
    libc/src/__support/OSUtil/gpu/CMakeLists.txt
    libc/src/__support/OSUtil/gpu/io.cpp
    libc/src/__support/OSUtil/gpu/io.h
    libc/src/__support/OSUtil/linux/CMakeLists.txt
    libc/src/__support/OSUtil/linux/io.h

Removed: 
    


################################################################################
diff  --git a/libc/src/__support/OSUtil/gpu/CMakeLists.txt b/libc/src/__support/OSUtil/gpu/CMakeLists.txt
index d1aa69604e515..01837d7ae0a2f 100644
--- a/libc/src/__support/OSUtil/gpu/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/gpu/CMakeLists.txt
@@ -8,5 +8,7 @@ add_object_library(
     io.h
   DEPENDS
     libc.src.__support.common
+    libc.src.__support.CPP.string_view
     libc.src.__support.RPC.rpc_client
+    libc.src.string.memory_utils.memcpy_implementation
 )

diff  --git a/libc/src/__support/OSUtil/gpu/io.cpp b/libc/src/__support/OSUtil/gpu/io.cpp
index dcc3bf76ec068..2570a1bfc25b4 100644
--- a/libc/src/__support/OSUtil/gpu/io.cpp
+++ b/libc/src/__support/OSUtil/gpu/io.cpp
@@ -8,23 +8,40 @@
 
 #include "io.h"
 
+#include "src/__support/CPP/string_view.h"
 #include "src/__support/RPC/rpc_client.h"
-#include "src/string/string_utils.h"
+#include "src/string/memory_utils/memcpy_implementations.h"
 
 namespace __llvm_libc {
 
-void write_to_stderr(const char *msg) {
-  uint64_t length = internal::string_length(msg) + 1;
-  uint64_t buffer_len = sizeof(rpc::Buffer) - sizeof(uint64_t);
-  for (uint64_t i = 0; i < length; i += buffer_len) {
-    rpc::client.run(
-        [&](rpc::Buffer *buffer) {
-          buffer->data[0] = rpc::Opcode::PRINT_TO_STDERR;
-          inline_memcpy(reinterpret_cast<char *>(&buffer->data[1]), &msg[i],
-                        (length > buffer_len ? buffer_len : length));
-        },
-        [](rpc::Buffer *) { /* void */ });
+namespace internal {
+
+static constexpr size_t BUFFER_SIZE = sizeof(rpc::Buffer) - sizeof(uint64_t);
+static constexpr size_t MAX_STRING_SIZE = BUFFER_SIZE;
+
+LIBC_INLINE void send_null_terminated(cpp::string_view src) {
+  rpc::client.run(
+      [&](rpc::Buffer *buffer) {
+        buffer->data[0] = rpc::Opcode::PRINT_TO_STDERR;
+        char *data = reinterpret_cast<char *>(&buffer->data[1]);
+        inline_memcpy(data, src.data(), src.size());
+        data[src.size()] = '\0';
+      },
+      [](rpc::Buffer *) { /* void */ });
+}
+
+} // namespace internal
+
+void write_to_stderr(cpp::string_view msg) {
+  bool send_empty_string = true;
+  for (; !msg.empty();) {
+    const auto chunk = msg.substr(0, internal::MAX_STRING_SIZE);
+    internal::send_null_terminated(chunk);
+    msg.remove_prefix(chunk.size());
+    send_empty_string = false;
   }
+  if (send_empty_string)
+    internal::send_null_terminated("");
 }
 
 } // namespace __llvm_libc

diff  --git a/libc/src/__support/OSUtil/gpu/io.h b/libc/src/__support/OSUtil/gpu/io.h
index e9a4ebf82a111..a27d1a114506f 100644
--- a/libc/src/__support/OSUtil/gpu/io.h
+++ b/libc/src/__support/OSUtil/gpu/io.h
@@ -9,9 +9,16 @@
 #ifndef LLVM_LIBC_SRC_SUPPORT_OSUTIL_GPU_IO_H
 #define LLVM_LIBC_SRC_SUPPORT_OSUTIL_GPU_IO_H
 
+#include "src/__support/CPP/string_view.h"
+#include "src/__support/macros/attributes.h" // LIBC_INLINE
+
 namespace __llvm_libc {
 
-void write_to_stderr(const char *msg);
+void write_to_stderr(cpp::string_view msg);
+
+LIBC_INLINE void write_to_stderr(const char *msg) {
+  write_to_stderr(cpp::string_view(msg));
+}
 
 } // namespace __llvm_libc
 

diff  --git a/libc/src/__support/OSUtil/linux/CMakeLists.txt b/libc/src/__support/OSUtil/linux/CMakeLists.txt
index 7499ef8d0648f..c27f9be746489 100644
--- a/libc/src/__support/OSUtil/linux/CMakeLists.txt
+++ b/libc/src/__support/OSUtil/linux/CMakeLists.txt
@@ -13,4 +13,5 @@ add_header_library(
   DEPENDS
     .${LIBC_TARGET_ARCHITECTURE}.linux_${LIBC_TARGET_ARCHITECTURE}_util
     libc.src.__support.common
+    libc.src.__support.CPP.string_view
 )

diff  --git a/libc/src/__support/OSUtil/linux/io.h b/libc/src/__support/OSUtil/linux/io.h
index 30180b946e7c3..cf387eb744472 100644
--- a/libc/src/__support/OSUtil/linux/io.h
+++ b/libc/src/__support/OSUtil/linux/io.h
@@ -9,16 +9,15 @@
 #ifndef LLVM_LIBC_SRC_SUPPORT_OSUTIL_LINUX_IO_H
 #define LLVM_LIBC_SRC_SUPPORT_OSUTIL_LINUX_IO_H
 
-#include "src/string/string_utils.h"
+#include "src/__support/CPP/string_view.h"
 #include "syscall.h" // For internal syscall function.
 
 #include <sys/syscall.h> // For syscall numbers.
 
 namespace __llvm_libc {
 
-LIBC_INLINE void write_to_stderr(const char *msg) {
-  __llvm_libc::syscall_impl(SYS_write, 2 /* stderr */, msg,
-                            internal::string_length(msg));
+LIBC_INLINE void write_to_stderr(cpp::string_view msg) {
+  __llvm_libc::syscall_impl(SYS_write, 2 /* stderr */, msg.data(), msg.size());
 }
 
 } // namespace __llvm_libc