User Details
- User Since
- Oct 28 2014, 11:30 AM (401 w, 20 h)
Mon, Jun 27
Fri, Jun 24
I have landed this now unblock other work. I am happy to do follow up changes as required.
Thu, Jun 23
Wed, Jun 22
Remove a now empty directory.
Tue, Jun 21
Address comments.
Sun, Jun 19
Fri, Jun 17
Thu, Jun 16
Wed, Jun 15
Nit: You should probably keep the addition of vfprintf_internal in the other patch.
Tue, Jun 14
When I do objdump -dr <path/to/libllvmlibc.a>, I see this:
Ah, never mind. I missed that you attached the archive. I will dig more into it.
That your libllvmlibc.a includes many syscall dependent entrypoints, and that your patch does not include a RISC-V syscall implementation, I am not confident that this is being tested in the correct way. As I requested in an earlier comment, I would like to see the ELF header for one of the object files in libllvmlibc.a. For example, can your share what you see when you run
Mon, Jun 13
Can you share the elf header for one of the object files in libllvmlibc.a. Also, can you share the list of files in libllvmlibc.a? The way to do it would be do:
Also, please provide a plan for CI builders.
I see a large number of syscall dependent entrypoints in your entrypoints.txt file. Without a RISCV syscall implementation they would fail. Did you run check-libc at all? If yes, and if you did get a clean run, can you share the output?
Can someone explain LLVM_LIBC_CACHELINE_SIZE ?
Sun, Jun 12
As @lntue has pointed out, the way start a new port would be to start with an entrypoints.txt and stick to default build. Default build is the mode in which one uses headers from the system libc. You can copy the entrypoints.txt for aarch64 and start peeling away entrypoints until you are able to build and test the remaining. In one of your messages, I see that you are hitting an errors with fenv functions. Remove them from your entrypoints.txt as they are target CPU specific implementations. To keep it easy, start with only string.h and math.h functions.
Fri, Jun 10
Nit: The changes to sqrt*.cpp and str_to_float.h are not related - please land them separately as "Obvious".
Remove debug messages.
Thu, Jun 9
LGTM modulo one comment about the NBF operation.
Good to go after the comments addressed.
Wed, Jun 8
Feel free to submit after addressing the comments.
The idea of "split point" makes it almost equivalent to the split approach I proposed. Taking a step back and looking at it objectively, consider that once a file is in a buffering mode, it is in that mode for its life time. Likewise, FBF is a mode by itself but LBF is FBF plus something else. To reflect these conceptual notions in code, we should use the approach of building LBF over FBF. That will keep FBF and LBF implementations simple and focused. Quantitatively also, the current state of this patch demonstrates that mixing makes it more complicated than it was previously.
Feel free to submit now.
Feel free to submit now.
Feel free to submit now.
Tue, Jun 7
While this is probably OK logically, it is way harder to reason than before. I am not yet convinced that this idea of flush_point and mixing line buffering and fully buffering is better.
LGTM - But, if some of the changes to the tests are unrelated, then do it separately.
Can we hold off on landing this until we resolve the bot problems?
Mon, Jun 6
As with the other patch, while this is accepted, please hold-off on submitting until further notice here.
While accepted, please hold-off on submission until further notice. Our bots have been taken off-line for maintenance. Once we bring them back, I will update this patch.
Jun 5 2022
Thanks a lot for the patch. It seems like it includes a bunch of cleanups / "better this way" items not related to the main goal of the patch. Can you please separate out those parts in to a different patch so that we can keep the review focused? Also, we cannot strictly use std C++ headers. So, no cerrno also. Include errno.h instead.