This is an archive of the discontinued LLVM Phabricator instance.

[libc][docs] Add doc for libc string functions
ClosedPublic

Authored by michaelrj on Apr 12 2022, 4:26 PM.

Details

Summary

This patch adds a document describing the status of the string functions
in LLVM-libc.

Diff Detail

Event Timeline

michaelrj created this revision.Apr 12 2022, 4:26 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 12 2022, 4:26 PM
michaelrj requested review of this revision.Apr 12 2022, 4:26 PM

Thanks for doing this! I do want to pull all of the impllemented/unimplemented functions into a list like the C++ folks have it, but this is nice to have.

It would also be nice to check to see if the _s functions are still being considered for removal and include that information here. http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm

jeffbailey accepted this revision.Apr 12 2022, 4:33 PM
This revision is now accepted and ready to land.Apr 12 2022, 4:33 PM
gchatelet added inline comments.Apr 13 2022, 2:28 AM
libc/docs/strings.rst
35–41

Outside of Google EXEgesis mostly refers to llvm-exegesis. It is not really connected to string functions (even though it's been developed by the same people).
Maybe call the section Primary memory function without description and leave the TODO assigned to me (omitting the exegesis word).

45

In this document, the Designed and Implemented columns are always equal. Maybe there is more value in telling which architectures are actually implemented ?

104

Maybe this should go into an Additional Functions section?

michaelrj updated this revision to Diff 422681.Apr 13 2022, 3:56 PM
michaelrj marked 2 inline comments as done.

addressed comments.

libc/docs/strings.rst
45

Most of these string functions are completely platform independent, since they work on individual bytes at a time, so giving all of them architecture specific statuses feels redundant. On the other hand, the memory functions and the conversion functions do have possible platform differences, so tracking them could be helpful. I think the biggest problem is that all of these functions already have versions for all of our supported platforms, so no matter how we slice it the table will just say "Done" everywhere, except for the functions that we haven't started.

104

I like them here because it puts them before the functions that aren't implemented yet.

sivachandra accepted this revision.Apr 13 2022, 4:31 PM
sivachandra added inline comments.
libc/docs/strings.rst
45

Perhaps a more useful way to present this is to just say whether an implementation is available or not. So, just one column Available under which you say YES if available, nothing if not. You can potentially add a Notes column in future to add additional notes if required. For a reader not involved in the development, that something has been designed or not does not help much. A person interested in contributing can engage on discourse et al to gather more information.

gchatelet added inline comments.Apr 14 2022, 4:46 AM
libc/docs/strings.rst
45

Yep in that case a single column is enough.

michaelrj marked 3 inline comments as done.

address comments and add performance information

add more specifics on performance comparison

Still LGTM with the suggested inline fix.

libc/docs/strings.rst
85

s/Designed/Available everywhere?

Still LGTM with the suggested inline fix.

Did you say something about endianness? Or is it X86 + aarch64 only?

michaelrj marked an inline comment as done.

address final comments

libc/docs/strings.rst
85

I went with Available over Available Everywhere since it allows for specifying which platforms it is available on if that's not everywhere.

This revision was landed with ongoing or failed builds.Apr 14 2022, 1:03 PM
This revision was automatically updated to reflect the committed changes.
sivachandra added inline comments.Apr 14 2022, 1:16 PM
libc/docs/strings.rst
85

Ah, I didn't quite understand when @tschuett made that comment. I did not intend to suggest replacing "Designed" with "Available everywhere" but was trying to say the "Designed" should be replaced with "Available" everywhere relevant.