Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc] Implement the 'ungetc' function on the GPU #69248

Merged
merged 1 commit into from
Oct 17, 2023
Merged

Conversation

jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Oct 16, 2023

Summary:
This function follows closely with the pattern of all the other
functions. That is, making a new opcode and forwarding the call to the
host. However, this also required modifying the test somewhat. It seems
that not all libc implementations follow the same error rules as are
tested here, and it is not explicit in the standard, so we simply
disable these EOF checks when targeting the GPU.

Summary:
This function follows closely with the pattern of all the other
functions. That is, making a new opcode and forwarding the call to the
host. However, this also required modifying the test somewhat. It seems
that not all `libc` implementations follow the same error rules as are
tested here, and it is not explicit in the standard, so we simply
disable these EOF checks when targeting the GPU.
@llvmbot
Copy link
Member

llvmbot commented Oct 16, 2023

@llvm/pr-subscribers-libc

Author: Joseph Huber (jhuber6)

Changes

Summary:
This function follows closely with the pattern of all the other
functions. That is, making a new opcode and forwarding the call to the
host. However, this also required modifying the test somewhat. It seems
that not all libc implementations follow the same error rules as are
tested here, and it is not explicit in the standard, so we simply
disable these EOF checks when targeting the GPU.


Full diff: https://github.com/llvm/llvm-project/pull/69248.diff

10 Files Affected:

  • (modified) libc/config/gpu/entrypoints.txt (+1)
  • (modified) libc/docs/gpu/support.rst (+1)
  • (modified) libc/include/llvm-libc-types/rpc_opcodes_t.h (+1)
  • (modified) libc/src/stdio/CMakeLists.txt (+1-12)
  • (modified) libc/src/stdio/generic/CMakeLists.txt (+12)
  • (renamed) libc/src/stdio/generic/ungetc.cpp ()
  • (modified) libc/src/stdio/gpu/CMakeLists.txt (+11)
  • (added) libc/src/stdio/gpu/ungetc.cpp (+29)
  • (modified) libc/test/src/stdio/ungetc_test.cpp (+8)
  • (modified) libc/utils/gpu/server/rpc_server.cpp (+7)
diff --git a/libc/config/gpu/entrypoints.txt b/libc/config/gpu/entrypoints.txt
index ad68216a76b9429..731508088cb6f8b 100644
--- a/libc/config/gpu/entrypoints.txt
+++ b/libc/config/gpu/entrypoints.txt
@@ -104,6 +104,7 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdio.fgetc
     libc.src.stdio.getc
     libc.src.stdio.getchar
+    libc.src.stdio.ungetc
     libc.src.stdio.stdin
     libc.src.stdio.stdout
     libc.src.stdio.stderr
diff --git a/libc/docs/gpu/support.rst b/libc/docs/gpu/support.rst
index fd27273ed562e49..806af5f219dfb43 100644
--- a/libc/docs/gpu/support.rst
+++ b/libc/docs/gpu/support.rst
@@ -134,6 +134,7 @@ ftell          |check|    |check|
 fflush         |check|    |check|
 fgetc          |check|    |check|
 fgets          |check|    |check|
+ungetc         |check|    |check|
 getc           |check|    |check|
 getchar        |check|    |check|
 puts           |check|    |check|
diff --git a/libc/include/llvm-libc-types/rpc_opcodes_t.h b/libc/include/llvm-libc-types/rpc_opcodes_t.h
index 61e17756fa64776..2fd318f06a7db16 100644
--- a/libc/include/llvm-libc-types/rpc_opcodes_t.h
+++ b/libc/include/llvm-libc-types/rpc_opcodes_t.h
@@ -29,6 +29,7 @@ typedef enum {
   RPC_FSEEK,
   RPC_FTELL,
   RPC_FFLUSH,
+  RPC_UNGETC,
   RPC_LAST = 0xFFFF,
 } rpc_opcode_t;
 
diff --git a/libc/src/stdio/CMakeLists.txt b/libc/src/stdio/CMakeLists.txt
index 169bc592dee4885..380474ce2711804 100644
--- a/libc/src/stdio/CMakeLists.txt
+++ b/libc/src/stdio/CMakeLists.txt
@@ -54,18 +54,6 @@ add_entrypoint_object(
     libc.src.__support.File.platform_file
 )
 
-add_entrypoint_object(
-  ungetc
-  SRCS
-    ungetc.cpp
-  HDRS
-    ungetc.h
-  DEPENDS
-    libc.include.stdio
-    libc.src.__support.File.file
-    libc.src.__support.File.platform_file
-)
-
 add_entrypoint_object(
   fopencookie
   SRCS
@@ -286,6 +274,7 @@ add_stdio_entrypoint_object(getc_unlocked)
 add_stdio_entrypoint_object(getchar)
 add_stdio_entrypoint_object(getchar_unlocked)
 add_stdio_entrypoint_object(fgets)
+add_stdio_entrypoint_object(ungetc)
 add_stdio_entrypoint_object(stdin)
 add_stdio_entrypoint_object(stdout)
 add_stdio_entrypoint_object(stderr)
diff --git a/libc/src/stdio/generic/CMakeLists.txt b/libc/src/stdio/generic/CMakeLists.txt
index 282d056bba71299..2ecef879eb4bbfe 100644
--- a/libc/src/stdio/generic/CMakeLists.txt
+++ b/libc/src/stdio/generic/CMakeLists.txt
@@ -342,6 +342,18 @@ add_entrypoint_object(
     libc.src.__support.File.platform_file
 )
 
+add_entrypoint_object(
+  ungetc
+  SRCS
+    ungetc.cpp
+  HDRS
+    ../ungetc.h
+  DEPENDS
+    libc.include.stdio
+    libc.src.__support.File.file
+    libc.src.__support.File.platform_file
+)
+
 add_entrypoint_object(
   stdin
   SRCS
diff --git a/libc/src/stdio/ungetc.cpp b/libc/src/stdio/generic/ungetc.cpp
similarity index 100%
rename from libc/src/stdio/ungetc.cpp
rename to libc/src/stdio/generic/ungetc.cpp
diff --git a/libc/src/stdio/gpu/CMakeLists.txt b/libc/src/stdio/gpu/CMakeLists.txt
index 047b68931bce5c3..1b1e2a903cc0b9b 100644
--- a/libc/src/stdio/gpu/CMakeLists.txt
+++ b/libc/src/stdio/gpu/CMakeLists.txt
@@ -251,6 +251,17 @@ add_entrypoint_object(
     .ferror
 )
 
+add_entrypoint_object(
+  ungetc
+  SRCS
+    ungetc.cpp
+  HDRS
+    ../ungetc.h
+  DEPENDS
+    libc.include.stdio
+    .gpu_file
+)
+
 add_entrypoint_object(
   stdin
   SRCS
diff --git a/libc/src/stdio/gpu/ungetc.cpp b/libc/src/stdio/gpu/ungetc.cpp
new file mode 100644
index 000000000000000..373164a0c53a326
--- /dev/null
+++ b/libc/src/stdio/gpu/ungetc.cpp
@@ -0,0 +1,29 @@
+//===-- Implementation of ungetc ------------------------------------------===//
+//
+// 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/ungetc.h"
+#include "file.h"
+
+#include <stdio.h>
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(int, ungetc, (int c, ::FILE *stream)) {
+  int ret;
+  rpc::Client::Port port = rpc::client.open<RPC_UNGETC>();
+  port.send_and_recv(
+      [=](rpc::Buffer *buffer) {
+        buffer->data[0] = c;
+        buffer->data[1] = file::from_stream(stream);
+      },
+      [&](rpc::Buffer *buffer) { ret = static_cast<int>(buffer->data[0]); });
+  port.close();
+  return ret;
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/test/src/stdio/ungetc_test.cpp b/libc/test/src/stdio/ungetc_test.cpp
index 75eecc87ef265ff..c98995ff0811bb3 100644
--- a/libc/test/src/stdio/ungetc_test.cpp
+++ b/libc/test/src/stdio/ungetc_test.cpp
@@ -24,12 +24,16 @@ TEST(LlvmLibcUngetcTest, UngetAndReadBack) {
   constexpr size_t CONTENT_SIZE = sizeof(CONTENT);
   ASSERT_EQ(CONTENT_SIZE,
             LIBC_NAMESPACE::fwrite(CONTENT, 1, CONTENT_SIZE, file));
+#ifndef LIBC_TARGET_ARCH_IS_GPU // Behavior varies between libc implementations.
   // Cannot unget to an un-readable file.
   ASSERT_EQ(EOF, LIBC_NAMESPACE::ungetc('1', file));
+#endif
   ASSERT_EQ(0, LIBC_NAMESPACE::fclose(file));
 
   file = LIBC_NAMESPACE::fopen(FILENAME, "r+");
   ASSERT_FALSE(file == nullptr);
+  // Calling with an EOF should always return EOF without doing anything.
+  ASSERT_EQ(EOF, LIBC_NAMESPACE::ungetc(EOF, file));
   char c;
   ASSERT_EQ(LIBC_NAMESPACE::fread(&c, 1, 1, file), size_t(1));
   ASSERT_EQ(c, CONTENT[0]);
@@ -43,8 +47,10 @@ TEST(LlvmLibcUngetcTest, UngetAndReadBack) {
   // ungetc should not fail after a seek operation.
   int unget_char = 'z';
   ASSERT_EQ(unget_char, LIBC_NAMESPACE::ungetc(unget_char, file));
+#ifndef LIBC_TARGET_ARCH_IS_GPU // Behavior varies between libc implementations.
   // Another unget should fail.
   ASSERT_EQ(EOF, LIBC_NAMESPACE::ungetc(unget_char, file));
+#endif
   // ungetting a char at the beginning of the file will allow us to fetch
   // one additional character.
   char new_data[CONTENT_SIZE + 1];
@@ -53,8 +59,10 @@ TEST(LlvmLibcUngetcTest, UngetAndReadBack) {
   ASSERT_STREQ("zabcdef", new_data);
 
   ASSERT_EQ(size_t(1), LIBC_NAMESPACE::fwrite("x", 1, 1, file));
+#ifndef LIBC_TARGET_ARCH_IS_GPU // Behavior varies between libc implementations.
   // unget should fail after a write operation.
   ASSERT_EQ(EOF, LIBC_NAMESPACE::ungetc('1', file));
+#endif
 
   ASSERT_EQ(0, LIBC_NAMESPACE::fclose(file));
 }
diff --git a/libc/utils/gpu/server/rpc_server.cpp b/libc/utils/gpu/server/rpc_server.cpp
index 1c1c9f1ae9e6b5d..0550115f7cd1a1c 100644
--- a/libc/utils/gpu/server/rpc_server.cpp
+++ b/libc/utils/gpu/server/rpc_server.cpp
@@ -186,6 +186,13 @@ struct Server {
       });
       break;
     }
+    case RPC_UNGETC: {
+      port->recv_and_send([](rpc::Buffer *buffer) {
+        buffer->data[0] = ungetc(static_cast<int>(buffer->data[0]),
+                                 file::to_stream(buffer->data[1]));
+      });
+      break;
+    }
     case RPC_NOOP: {
       port->recv([](rpc::Buffer *) {});
       break;

Copy link
Contributor

@lntue lntue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM on the libc side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants