Skip to content

Conversation

petrhosek
Copy link
Member

This avoids vendor having to provide one in order to link the libc.

This avoids vendor having to provide one in order to link the libc.
@llvmbot
Copy link
Member

llvmbot commented Mar 28, 2024

@llvm/pr-subscribers-libc

Author: Petr Hosek (petrhosek)

Changes

This avoids vendor having to provide one in order to link the libc.


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

2 Files Affected:

  • (modified) libc/src/__support/OSUtil/baremetal/io.cpp (+1-1)
  • (modified) libc/src/__support/OSUtil/baremetal/quick_exit.cpp (+3-1)
diff --git a/libc/src/__support/OSUtil/baremetal/io.cpp b/libc/src/__support/OSUtil/baremetal/io.cpp
index 347c7d405b0a9f..61d12aa7865700 100644
--- a/libc/src/__support/OSUtil/baremetal/io.cpp
+++ b/libc/src/__support/OSUtil/baremetal/io.cpp
@@ -11,7 +11,7 @@
 #include "src/__support/CPP/string_view.h"
 
 // This is intended to be provided by the vendor.
-extern "C" void __llvm_libc_log_write(const char *msg, size_t len);
+extern "C" [[gnu::weak]] void __llvm_libc_log_write(const char *, size_t) {}
 
 namespace LIBC_NAMESPACE {
 
diff --git a/libc/src/__support/OSUtil/baremetal/quick_exit.cpp b/libc/src/__support/OSUtil/baremetal/quick_exit.cpp
index 5b6fcf42341ebb..be29e83b68ce67 100644
--- a/libc/src/__support/OSUtil/baremetal/quick_exit.cpp
+++ b/libc/src/__support/OSUtil/baremetal/quick_exit.cpp
@@ -9,7 +9,9 @@
 #include "src/__support/OSUtil/quick_exit.h"
 
 // This is intended to be provided by the vendor.
-extern "C" [[noreturn]] void __llvm_libc_quick_exit(int status);
+extern "C" [[gnu::weak]] [[noreturn]] void __llvm_libc_quick_exit(int) {
+  __builtin_trap();
+}
 
 namespace LIBC_NAMESPACE {
 

@petrhosek
Copy link
Member Author

I'm undecided whether this is desirable or not. On one hand it makes it easier to drop in LLVM libc without any changes to the existing codebase. On the other hand, we shouldn't be silently ignoring errors and want users to implement these functions whenever possible.

Copy link
Contributor

@jhuber6 jhuber6 left a comment

Choose a reason for hiding this comment

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

LG, just keep in mind that weak symbols do not extract from static archives using the normal semantics. So if these definitions are provided by a static library they won't get pulled in unless using --whole-archive or something else extracts them.

@nickdesaulniers
Copy link
Member

I'm undecided whether this is desirable or not. On one hand it makes it easier to drop in LLVM libc without any changes to the existing codebase. On the other hand, we shouldn't be silently ignoring errors and want users to implement these functions whenever possible.

I think it would be better to retain the existing behavior of explicit linkage failure, but we should instead write docs about what minimal runtime elements users MUST implement to use llvm-libc on baremetal.

https://libc.llvm.org/ has a tab "libc for GPUs". Right under it should be "libc for baremetal" which should list what functions need to be provided, and what their semantics need to be.

@petrhosek petrhosek closed this Jul 20, 2024
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.

4 participants