[libc] Implement 'getenv' on the GPU target #102376

jhuber6 · 2024-08-07T21:45:54Z

Summary:
This patch implements 'getenv'. I was torn on how to implement this,
since realistically we only have access to this environment pointer in
the "loader" interface. An alternative would be to use an RPC call every
time, but I think that's overkill for what this will be used for. A
better solution is just to emit a common DataEnvironment that contains
all of the host visible resources to initialize. Right now this is the
env_ptr, clock_freq, and rpc_client.

I did this by making the app.h interface that Linux uses more general,
could possibly move that into a separate patch, but I figured it's
easier to see with the usage.

Summary: This patch implements 'getenv'. I was torn on how to implement this, since realistically we only have access to this environment pointer in the "loader" interface. An alternative would be to use an RPC call every time, but I think that's overkill for what this will be used for. A better solution is just to emit a common `DataEnvironment` that contains all of the host visible resources to initialize. Right now this is the `env_ptr`, `clock_freq`, and `rpc_client`. I did this by making the `app.h` interface that Linux uses more general, could possibly move that into a separate patch, but I figured it's easier to see with the usage.

llvmbot · 2024-08-07T21:46:23Z

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-libc

Author: Joseph Huber (jhuber6)

Changes

Summary:
This patch implements 'getenv'. I was torn on how to implement this,
since realistically we only have access to this environment pointer in
the "loader" interface. An alternative would be to use an RPC call every
time, but I think that's overkill for what this will be used for. A
better solution is just to emit a common DataEnvironment that contains
all of the host visible resources to initialize. Right now this is the
env_ptr, clock_freq, and rpc_client.

I did this by making the app.h interface that Linux uses more general,
could possibly move that into a separate patch, but I figured it's
easier to see with the usage.

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

22 Files Affected:

(modified) libc/config/CMakeLists.txt (+7-3)
(added) libc/config/app.h (+20)
(added) libc/config/gpu/app.h (+28)
(modified) libc/config/gpu/entrypoints.txt (+1)
(removed) libc/config/linux/CMakeLists.txt (-7)
(modified) libc/src/__support/threads/linux/CMakeLists.txt (+1-1)
(modified) libc/src/__support/threads/linux/thread.cpp (+1-1)
(modified) libc/src/stdlib/CMakeLists.txt (+1-1)
(modified) libc/src/stdlib/getenv.cpp (+1-1)
(modified) libc/src/sys/auxv/linux/CMakeLists.txt (+1-1)
(modified) libc/src/sys/auxv/linux/getauxval.cpp (+1-1)
(modified) libc/startup/gpu/CMakeLists.txt (+1-1)
(modified) libc/startup/gpu/amdgpu/CMakeLists.txt (+1)
(modified) libc/startup/gpu/amdgpu/start.cpp (+5)
(modified) libc/startup/gpu/nvptx/CMakeLists.txt (+1)
(modified) libc/startup/gpu/nvptx/start.cpp (+6)
(modified) libc/startup/linux/CMakeLists.txt (+1-1)
(modified) libc/startup/linux/aarch64/CMakeLists.txt (+2-2)
(modified) libc/startup/linux/do_start.h (+1-1)
(modified) libc/startup/linux/riscv/CMakeLists.txt (+2-2)
(modified) libc/startup/linux/x86_64/CMakeLists.txt (+2-2)
(modified) libc/test/integration/src/stdlib/CMakeLists.txt (-1)

diff --git a/libc/config/CMakeLists.txt b/libc/config/CMakeLists.txt
index 853854b03be4e..cf38ae3eed726 100644
--- a/libc/config/CMakeLists.txt
+++ b/libc/config/CMakeLists.txt
@@ -1,3 +1,7 @@
-#TODO: Properly select the correct subdirectory.
-
-add_subdirectory(linux)
+add_header_library(
+  app_h
+  HDRS
+    app.h
+  DEPENDS
+    libc.src.__support.common
+)
diff --git a/libc/config/app.h b/libc/config/app.h
new file mode 100644
index 0000000000000..27f4141d80c4b
--- /dev/null
+++ b/libc/config/app.h
@@ -0,0 +1,20 @@
+//===-- Classes to capture properites of applications -----------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_CONFIG_APP_H
+#define LLVM_LIBC_CONFIG_APP_H
+
+#include "src/__support/macros/properties/architectures.h"
+
+#if defined(LIBC_TARGET_ARCH_IS_GPU)
+#include "gpu/app.h"
+#elif defined(__linux__)
+#include "linux/app.h"
+#endif
+
+#endif // LLVM_LIBC_CONFIG_APP_H
diff --git a/libc/config/gpu/app.h b/libc/config/gpu/app.h
new file mode 100644
index 0000000000000..148c51b702203
--- /dev/null
+++ b/libc/config/gpu/app.h
@@ -0,0 +1,28 @@
+//===-- Classes to capture properites of GPU applications -------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_CONFIG_GPU_APP_H
+#define LLVM_LIBC_CONFIG_GPU_APP_H
+
+#include "src/__support/macros/config.h"
+#include "src/__support/macros/properties/architectures.h"
+
+#include <stdint.h>
+
+namespace LIBC_NAMESPACE_DECL {
+
+// TODO: Move other global values here and export them to the host.
+struct DataEnvironment {
+  uintptr_t *env_ptr;
+};
+
+extern DataEnvironment app;
+
+} // namespace LIBC_NAMESPACE_DECL
+
+#endif // LLVM_LIBC_CONFIG_GPU_APP_H
diff --git a/libc/config/gpu/entrypoints.txt b/libc/config/gpu/entrypoints.txt
index e1a16a3b68878..3e0e6aff55245 100644
--- a/libc/config/gpu/entrypoints.txt
+++ b/libc/config/gpu/entrypoints.txt
@@ -167,6 +167,7 @@ set(TARGET_LIBC_ENTRYPOINTS
     libc.src.stdlib.strtoull
     libc.src.stdlib.at_quick_exit
     libc.src.stdlib.quick_exit
+    libc.src.stdlib.getenv
 
     # TODO: Implement these correctly
     libc.src.stdlib.aligned_alloc
diff --git a/libc/config/linux/CMakeLists.txt b/libc/config/linux/CMakeLists.txt
deleted file mode 100644
index cf38ae3eed726..0000000000000
--- a/libc/config/linux/CMakeLists.txt
+++ /dev/null
@@ -1,7 +0,0 @@
-add_header_library(
-  app_h
-  HDRS
-    app.h
-  DEPENDS
-    libc.src.__support.common
-)
diff --git a/libc/src/__support/threads/linux/CMakeLists.txt b/libc/src/__support/threads/linux/CMakeLists.txt
index c2f0ed0cb233d..b6796f40adce7 100644
--- a/libc/src/__support/threads/linux/CMakeLists.txt
+++ b/libc/src/__support/threads/linux/CMakeLists.txt
@@ -77,7 +77,7 @@ add_object_library(
     thread.cpp
   DEPENDS
     .futex_utils
-    libc.config.linux.app_h
+    libc.config.app_h
     libc.include.sys_syscall
     libc.include.fcntl
     libc.src.errno.errno
diff --git a/libc/src/__support/threads/linux/thread.cpp b/libc/src/__support/threads/linux/thread.cpp
index 36b4a88eba9b4..ee3f63fa3cde3 100644
--- a/libc/src/__support/threads/linux/thread.cpp
+++ b/libc/src/__support/threads/linux/thread.cpp
@@ -7,7 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/__support/threads/thread.h"
-#include "config/linux/app.h"
+#include "config/app.h"
 #include "src/__support/CPP/atomic.h"
 #include "src/__support/CPP/string_view.h"
 #include "src/__support/CPP/stringstream.h"
diff --git a/libc/src/stdlib/CMakeLists.txt b/libc/src/stdlib/CMakeLists.txt
index 29789f5e2adc2..ce12e66cf3e57 100644
--- a/libc/src/stdlib/CMakeLists.txt
+++ b/libc/src/stdlib/CMakeLists.txt
@@ -62,7 +62,7 @@ add_entrypoint_object(
   HDRS
     getenv.h
   DEPENDS
-    libc.config.linux.app_h
+  libc.config.app_h
 )
 
 add_entrypoint_object(
diff --git a/libc/src/stdlib/getenv.cpp b/libc/src/stdlib/getenv.cpp
index 6b1bb693a6d83..e6ef03fad5c51 100644
--- a/libc/src/stdlib/getenv.cpp
+++ b/libc/src/stdlib/getenv.cpp
@@ -7,7 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/stdlib/getenv.h"
-#include "config/linux/app.h"
+#include "config/app.h"
 #include "src/__support/CPP/string_view.h"
 #include "src/__support/common.h"
 #include "src/__support/macros/config.h"
diff --git a/libc/src/sys/auxv/linux/CMakeLists.txt b/libc/src/sys/auxv/linux/CMakeLists.txt
index 383c29eafda8d..4884184cc6053 100644
--- a/libc/src/sys/auxv/linux/CMakeLists.txt
+++ b/libc/src/sys/auxv/linux/CMakeLists.txt
@@ -11,7 +11,7 @@ add_entrypoint_object(
     libc.src.__support.threads.callonce
     libc.src.__support.common
     libc.src.errno.errno
-    libc.config.linux.app_h
+    libc.config.app_h
     libc.src.fcntl.open
     libc.src.unistd.read
     libc.src.unistd.close
diff --git a/libc/src/sys/auxv/linux/getauxval.cpp b/libc/src/sys/auxv/linux/getauxval.cpp
index bfa6b23b5ef91..236fd25698f65 100644
--- a/libc/src/sys/auxv/linux/getauxval.cpp
+++ b/libc/src/sys/auxv/linux/getauxval.cpp
@@ -7,7 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "src/sys/auxv/getauxval.h"
-#include "config/linux/app.h"
+#include "config/app.h"
 #include "src/__support/common.h"
 #include "src/__support/macros/config.h"
 #include "src/errno/libc_errno.h"
diff --git a/libc/startup/gpu/CMakeLists.txt b/libc/startup/gpu/CMakeLists.txt
index 5e5745063fc8c..9d0e0885dff93 100644
--- a/libc/startup/gpu/CMakeLists.txt
+++ b/libc/startup/gpu/CMakeLists.txt
@@ -34,7 +34,7 @@ function(add_startup_object name)
       RUNTIME_OUTPUT_DIRECTORY ${LIBC_LIBRARY_DIR}
       RUNTIME_OUTPUT_NAME ${name}.o)
     target_link_options(${fq_target_name}.exe PRIVATE
-                        "-nostdlib" "-flto" "-Wl,--lto-emit-llvm")
+                        "-r" "-nostdlib" "-flto" "-Wl,--lto-emit-llvm")
   endif()
 endfunction()
 
diff --git a/libc/startup/gpu/amdgpu/CMakeLists.txt b/libc/startup/gpu/amdgpu/CMakeLists.txt
index 3ac104ee8ba94..b67a5a2cc89fb 100644
--- a/libc/startup/gpu/amdgpu/CMakeLists.txt
+++ b/libc/startup/gpu/amdgpu/CMakeLists.txt
@@ -3,6 +3,7 @@ add_startup_object(
   SRC
     start.cpp
   DEPENDS
+    libc.config.app_h
     libc.src.__support.RPC.rpc_client
     libc.src.__support.GPU.utils
     libc.src.stdlib.exit
diff --git a/libc/startup/gpu/amdgpu/start.cpp b/libc/startup/gpu/amdgpu/start.cpp
index 6bda151023c8f..5aaa7e938d279 100644
--- a/libc/startup/gpu/amdgpu/start.cpp
+++ b/libc/startup/gpu/amdgpu/start.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "config/gpu/app.h"
 #include "src/__support/GPU/utils.h"
 #include "src/__support/RPC/rpc_client.h"
 #include "src/__support/macros/config.h"
@@ -16,6 +17,8 @@ extern "C" int main(int argc, char **argv, char **envp);
 
 namespace LIBC_NAMESPACE_DECL {
 
+DataEnvironment app;
+
 extern "C" uintptr_t __init_array_start[];
 extern "C" uintptr_t __init_array_end[];
 extern "C" uintptr_t __fini_array_start[];
@@ -40,6 +43,8 @@ static void call_fini_array_callbacks() {
 
 extern "C" [[gnu::visibility("protected"), clang::amdgpu_kernel]] void
 _begin(int argc, char **argv, char **env) {
+  __atomic_store_n(&LIBC_NAMESPACE::app.env_ptr,
+                   reinterpret_cast<uintptr_t *>(env), __ATOMIC_RELAXED);
   // We want the fini array callbacks to be run after other atexit
   // callbacks are run. So, we register them before running the init
   // array callbacks as they can potentially register their own atexit
diff --git a/libc/startup/gpu/nvptx/CMakeLists.txt b/libc/startup/gpu/nvptx/CMakeLists.txt
index 3ac104ee8ba94..b67a5a2cc89fb 100644
--- a/libc/startup/gpu/nvptx/CMakeLists.txt
+++ b/libc/startup/gpu/nvptx/CMakeLists.txt
@@ -3,6 +3,7 @@ add_startup_object(
   SRC
     start.cpp
   DEPENDS
+    libc.config.app_h
     libc.src.__support.RPC.rpc_client
     libc.src.__support.GPU.utils
     libc.src.stdlib.exit
diff --git a/libc/startup/gpu/nvptx/start.cpp b/libc/startup/gpu/nvptx/start.cpp
index b1ef944c4aa28..ef1e63e5161a6 100644
--- a/libc/startup/gpu/nvptx/start.cpp
+++ b/libc/startup/gpu/nvptx/start.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "config/gpu/app.h"
 #include "src/__support/GPU/utils.h"
 #include "src/__support/RPC/rpc_client.h"
 #include "src/__support/macros/config.h"
@@ -16,6 +17,8 @@ extern "C" int main(int argc, char **argv, char **envp);
 
 namespace LIBC_NAMESPACE_DECL {
 
+DataEnvironment app;
+
 extern "C" {
 // Nvidia's 'nvlink' linker does not provide these symbols. We instead need
 // to manually create them and update the globals in the loader implememtation.
@@ -46,6 +49,9 @@ static void call_fini_array_callbacks() {
 
 extern "C" [[gnu::visibility("protected"), clang::nvptx_kernel]] void
 _begin(int argc, char **argv, char **env) {
+  __atomic_store_n(&LIBC_NAMESPACE::app.env_ptr,
+                   reinterpret_cast<uintptr_t *>(env), __ATOMIC_RELAXED);
+
   // We want the fini array callbacks to be run after other atexit
   // callbacks are run. So, we register them before running the init
   // array callbacks as they can potentially register their own atexit
diff --git a/libc/startup/linux/CMakeLists.txt b/libc/startup/linux/CMakeLists.txt
index 336c5d0f6bfa2..71f187ca05f29 100644
--- a/libc/startup/linux/CMakeLists.txt
+++ b/libc/startup/linux/CMakeLists.txt
@@ -95,7 +95,7 @@ add_object_library(
   HDRS
     do_start.h
   DEPENDS
-    libc.config.linux.app_h
+    libc.config.app_h
     libc.include.sys_mman
     libc.include.sys_syscall
     libc.include.llvm-libc-macros.link_macros
diff --git a/libc/startup/linux/aarch64/CMakeLists.txt b/libc/startup/linux/aarch64/CMakeLists.txt
index 5ea6ae59abcb2..5564f0a8f687e 100644
--- a/libc/startup/linux/aarch64/CMakeLists.txt
+++ b/libc/startup/linux/aarch64/CMakeLists.txt
@@ -3,7 +3,7 @@ add_startup_object(
   SRC
     tls.cpp
   DEPENDS
-    libc.config.linux.app_h
+    libc.config.app_h
     libc.include.sys_mman
     libc.include.sys_syscall
     libc.src.__support.OSUtil.osutil
@@ -18,7 +18,7 @@ add_startup_object(
   SRC
     start.cpp
   DEPENDS
-    libc.config.linux.app_h
+    libc.config.app_h
   COMPILE_OPTIONS
     -fno-omit-frame-pointer
     -ffreestanding # To avoid compiler warnings about calling the main function.
diff --git a/libc/startup/linux/do_start.h b/libc/startup/linux/do_start.h
index dd41c9bd384e7..8fc8c3716f2ac 100644
--- a/libc/startup/linux/do_start.h
+++ b/libc/startup/linux/do_start.h
@@ -6,7 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "config/linux/app.h"
+#include "config/app.h"
 #include "src/__support/macros/config.h"
 
 namespace LIBC_NAMESPACE_DECL {
diff --git a/libc/startup/linux/riscv/CMakeLists.txt b/libc/startup/linux/riscv/CMakeLists.txt
index 3717784233c15..2a61f8289067d 100644
--- a/libc/startup/linux/riscv/CMakeLists.txt
+++ b/libc/startup/linux/riscv/CMakeLists.txt
@@ -3,7 +3,7 @@ add_startup_object(
   SRC
     tls.cpp
   DEPENDS
-    libc.config.linux.app_h
+    libc.config.app_h
     libc.include.sys_mman
     libc.include.sys_syscall
     libc.src.__support.OSUtil.osutil
@@ -18,7 +18,7 @@ add_startup_object(
   SRC
     start.cpp
   DEPENDS
-    libc.config.linux.app_h
+    libc.config.app_h
     libc.src.__support.macros.attributes
   COMPILE_OPTIONS
     -fno-omit-frame-pointer
diff --git a/libc/startup/linux/x86_64/CMakeLists.txt b/libc/startup/linux/x86_64/CMakeLists.txt
index 30da7ab4e1ec3..4f482eaf5d18e 100644
--- a/libc/startup/linux/x86_64/CMakeLists.txt
+++ b/libc/startup/linux/x86_64/CMakeLists.txt
@@ -3,7 +3,7 @@ add_startup_object(
   SRC
     tls.cpp
   DEPENDS
-    libc.config.linux.app_h
+    libc.config.app_h
     libc.include.sys_mman
     libc.include.sys_syscall
     libc.src.__support.OSUtil.osutil
@@ -20,7 +20,7 @@ add_startup_object(
   SRC
     start.cpp
   DEPENDS
-    libc.config.linux.app_h
+    libc.config.app_h
     libc.src.__support.macros.attributes
   COMPILE_OPTIONS
     -fno-stack-protector
diff --git a/libc/test/integration/src/stdlib/CMakeLists.txt b/libc/test/integration/src/stdlib/CMakeLists.txt
index 0985a80ce7a09..1efdf607defe9 100644
--- a/libc/test/integration/src/stdlib/CMakeLists.txt
+++ b/libc/test/integration/src/stdlib/CMakeLists.txt
@@ -13,4 +13,3 @@ add_integration_test(
     FRANCE=Paris
     GERMANY=Berlin
 )
-

SchrodingerZhu · 2024-08-08T04:52:06Z

libc/startup/gpu/CMakeLists.txt

@@ -34,7 +34,7 @@ function(add_startup_object name)
      RUNTIME_OUTPUT_DIRECTORY ${LIBC_LIBRARY_DIR}
      RUNTIME_OUTPUT_NAME ${name}.o)
    target_link_options(${fq_target_name}.exe PRIVATE


Why was GPU startup object named with .exe suffix

It's just a dummy target, needed to be unique and since it's an executable, it's .exe.

SchrodingerZhu

LGTM

jhuber6 requested review from lntue, michaelrj-google and SchrodingerZhu August 7, 2024 21:45

llvmbot added backend:AMDGPU libc labels Aug 7, 2024

SchrodingerZhu reviewed Aug 8, 2024

View reviewed changes

SchrodingerZhu approved these changes Aug 8, 2024

View reviewed changes

jhuber6 merged commit 1a92cc5 into llvm:main Aug 8, 2024
9 checks passed

jhuber6 deleted the getenv branch September 23, 2024 13:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc] Implement 'getenv' on the GPU target #102376

[libc] Implement 'getenv' on the GPU target #102376

Uh oh!

jhuber6 commented Aug 7, 2024

Uh oh!

llvmbot commented Aug 7, 2024 •

edited

Loading

Uh oh!

SchrodingerZhu Aug 8, 2024

Uh oh!

jhuber6 Aug 8, 2024

Uh oh!

SchrodingerZhu left a comment

Uh oh!

Uh oh!

Uh oh!

[libc] Implement 'getenv' on the GPU target #102376

[libc] Implement 'getenv' on the GPU target #102376

Uh oh!

Conversation

jhuber6 commented Aug 7, 2024

Uh oh!

llvmbot commented Aug 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SchrodingerZhu Aug 8, 2024

Choose a reason for hiding this comment

Uh oh!

jhuber6 Aug 8, 2024

Choose a reason for hiding this comment

Uh oh!

SchrodingerZhu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Aug 7, 2024 •

edited

Loading