llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.23k stars 11.65k forks source link

[libc] Unify location of baremetal output hooks #94685

Open michaelrj-google opened 3 months ago

michaelrj-google commented 3 months ago

In patch #94078 I added a new baremetal hook to allow for writing directly from printf to some equivalent of stdout. During review it was brought up that there exists a hook in https://github.com/llvm/llvm-project/blob/main/libc/src/__support/OSUtil/baremetal/io.cpp for writing to an equivalent of stderr. These are distinct enough I think they should both be included, but similar enough they should probably be in the same spot.

Since this is an external hook it doesn't technically matter where we put it, but there should be a unified list. To get that sort of unified list it would make sense to also create a write_to_stdout to match the existing write_to_stderr. Then the question arises of "how should write_to_stdout work on platforms other than baremetal?"

Option 1: Direct syscall to write to fd 0. Pro: Simple Cons: Completely ignores the file buffering mechanism, causing out of order output.

Option 2: Write to the stdout FILE object. Pro: Proper buffering behavior, we could use this in puts and putchar as well, meaning those don't need specialized baremetal versions. Con: Complexity around dependency ordering.

Regardless of if we go with one of these options or something else, it's out of scope for that original patch.

llvmbot commented 3 months ago

@llvm/issue-subscribers-libc

Author: Michael Jones (michaelrj-google)

In patch #94078 I added a new baremetal hook to allow for writing directly from printf to some equivalent of `stdout`. During review it was brought up that there exists a hook in https://github.com/llvm/llvm-project/blob/main/libc/src/__support/OSUtil/baremetal/io.cpp for writing to an equivalent of `stderr`. These are distinct enough I think they should both be included, but similar enough they should probably be in the same spot. Since this is an external hook it doesn't technically matter where we put it, but there should be a unified list. To get that sort of unified list it would make sense to also create a `write_to_stdout` to match the existing `write_to_stderr`. Then the question arises of "how should `write_to_stdout` work on platforms other than baremetal?" Option 1: Direct syscall to write to fd 0. Pro: Simple Cons: Completely ignores the file buffering mechanism, causing out of order output. Option 2: Write to the `stdout` `FILE` object. Pro: Proper buffering behavior, we could use this in `puts` and `putchar` as well, meaning those don't need specialized baremetal versions. Con: Complexity around dependency ordering. Regardless of if we go with one of these options or something else, it's out of scope for that original patch.