Skip to content

[libc] Add baremetal printf #94078

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.

Sign up for GitHub

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

Merged
michaelrj-google merged 3 commits into from
Jun 7, 2024
Merged

[libc] Add baremetal printf #94078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4d082a2
[libc] Add baremetal printf
michaelrj-google May 31, 2024
d3cc1f7
address comments
michaelrj-google Jun 4, 2024
93fe014
adjust comments
michaelrj-google Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 2 additions & 33 deletions libc/src/stdio/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,29 +159,6 @@ 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
libc.src.__support.File.platform_stdout
)
endif()

add_entrypoint_object(
printf
SRCS
printf.cpp
HDRS
printf.h
DEPENDS
${printf_deps}
)

add_entrypoint_object(
fprintf
SRCS
Expand Down Expand Up @@ -215,16 +192,6 @@ add_entrypoint_object(
libc.src.stdio.printf_core.writer
)

add_entrypoint_object(
vprintf
SRCS
vprintf.cpp
HDRS
vprintf.h
DEPENDS
${printf_deps}
)

add_entrypoint_object(
vfprintf
SRCS
Expand Down Expand Up @@ -286,6 +253,7 @@ add_stdio_entrypoint_object(fwrite)
add_stdio_entrypoint_object(fputc)
add_stdio_entrypoint_object(putc)
add_stdio_entrypoint_object(putchar)
add_stdio_entrypoint_object(printf)
add_stdio_entrypoint_object(fgetc)
add_stdio_entrypoint_object(fgetc_unlocked)
add_stdio_entrypoint_object(getc)
Expand All @@ -297,3 +265,4 @@ add_stdio_entrypoint_object(ungetc)
add_stdio_entrypoint_object(stdin)
add_stdio_entrypoint_object(stdout)
add_stdio_entrypoint_object(stderr)
add_stdio_entrypoint_object(vprintf)
12 changes: 12 additions & 0 deletions libc/src/stdio/baremetal/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,15 @@ add_entrypoint_object(
DEPENDS
libc.include.stdio
)

add_entrypoint_object(
printf
SRCS
printf.cpp
HDRS
../printf.h
DEPENDS
libc.src.stdio.printf_core.printf_main
libc.src.stdio.printf_core.writer
libc.src.__support.arg_list
)
57 changes: 57 additions & 0 deletions libc/src/stdio/baremetal/printf.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//===-- Implementation of printf for baremetal ------------------*- 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.h"
#include "src/__support/arg_list.h"
#include "src/stdio/printf_core/core_structs.h"
#include "src/stdio/printf_core/printf_main.h"
#include "src/stdio/printf_core/writer.h"

#include <stdarg.h>

// TODO(https://github.com/llvm/llvm-project/issues/94685) unify baremetal hooks

// This is intended to be provided by the vendor.
extern "C" size_t __llvm_libc_raw_write(const char *s, size_t size);
Copy link
Contributor

@jhuber6 jhuber6 May 31, 2024

Choose a reason for hiding this comment

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

How is this different from the write_to_stderr hook we already use? I guess the size is different. For the GPU I use FILE as a simple opaque handle basically, don't know if that's relevant here.

Copy link
Contributor Author

@michaelrj-google michaelrj-google May 31, 2024

Choose a reason for hiding this comment

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

I'd say raw_write and write_to_stderr are (in my mind) different in the same way stdout and stderr are. The intent for stdout is for normal output, whereas the intent for stderr is error messages. That being said, I should probably move this to OSUtil so that it's in the same place as write_to_stderr.

As for using FILE as an opaque handle, that probably would work, but the request from @petrhosek was for a printf that only writes to the provided function.

Copy link
Member

@petrhosek petrhosek Jun 1, 2024

Choose a reason for hiding this comment

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

write_to_stderr is a private C++ API, for vendor integration we need an internal C API. So far we only have __llvm_libc_log_write defined in baremetal OSUtil, and this change introduces __llvm_libc_raw_write. It's a question whether this API should be specific only to baremetal target or should be made more generic, but I'm fine starting with just baremetal and generalizing it as needed. It'd be also helpful to define all internal API in a single header that's extensively documented. There are also open questions around naming, versioning, prior art, etc. This is a broader topic that we should probably discuss on Discourse, but I'm fine landing this change in the meantime and refining the API later.

Regarding opaque FILE, for baremetal I think we'll need both. Most baremetal targets have some support for writing out characters, typically over serial. This would typically be used to implement printf (or similar API for logging). Many baremetal targets also support semihosting which allows file system access and I'd like to implement FILE for baremetal through semihosting. We could also use semihosting to implement printf (using SYS_WRITE syscall) but that has a downside of requiring a debugger and so I don't think it should be the default implementation.

Copy link
Contributor Author

@michaelrj-google michaelrj-google Jun 4, 2024

Choose a reason for hiding this comment

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

having thought about it some more, I think the best solution might be to add a write_to_stdout that can be used by not just printf but also puts and putchar in future. I'll update this PR with what I currently have and then try making a version with this new design so that we can compare.

Copy link
Contributor Author

@michaelrj-google michaelrj-google Jun 6, 2024

Choose a reason for hiding this comment

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

I messed around with the design a bit and realized it's out of scope for this patch. I've filed an issue to track this further (#94685), but for now I'm going to land this patch as-is.


namespace LIBC_NAMESPACE {

namespace {

LIBC_INLINE int raw_write_hook(cpp::string_view new_str, void *) {
size_t written = __llvm_libc_raw_write(new_str.data(), new_str.size());
if (written != new_str.size())
return printf_core::FILE_WRITE_ERROR;
return printf_core::WRITE_OK;
}

} // namespace

LLVM_LIBC_FUNCTION(int, printf, (const char *__restrict format, ...)) {
va_list vlist;
va_start(vlist, format);
internal::ArgList args(vlist); // This holder class allows for easier copying
// and pointer semantics, as well as handling
// destruction automatically.
va_end(vlist);
constexpr size_t BUFF_SIZE = 1024;
char buffer[BUFF_SIZE];

printf_core::WriteBuffer wb(buffer, BUFF_SIZE, &raw_write_hook, nullptr);
printf_core::Writer writer(&wb);

int retval = printf_core::printf_main(&writer, format, args);

int flushval = wb.overflow_write("");
if (flushval != printf_core::WRITE_OK)
retval = flushval;

return retval;
}

} // namespace LIBC_NAMESPACE
33 changes: 33 additions & 0 deletions libc/src/stdio/generic/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,39 @@ add_entrypoint_object(
libc.src.__support.File.platform_file
)

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
libc.src.__support.File.platform_stdout
)
endif()

add_entrypoint_object(
printf
SRCS
printf.cpp
HDRS
../printf.h
DEPENDS
${printf_deps}
)

add_entrypoint_object(
vprintf
SRCS
vprintf.cpp
HDRS
../vprintf.h
DEPENDS
${printf_deps}
)

add_entrypoint_object(
fgets
SRCS
Expand Down
0 libc/src/stdio/printf.cpp → libc/src/stdio/generic/printf.cpp
View file
Open in desktop
File renamed without changes.
0 libc/src/stdio/vprintf.cpp → libc/src/stdio/generic/vprintf.cpp
View file
Open in desktop
File renamed without changes.