This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Inline small constructors into basic_string
ClosedPublic

Authored by philnik on Feb 22 2023, 11:16 AM.

Details

Summary

This allows the compiler to inline the constructors.

Diff Detail

Event Timeline

philnik created this revision.Feb 22 2023, 11:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 22 2023, 11:16 AM
philnik requested review of this revision.Feb 22 2023, 11:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 22 2023, 11:16 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Mordante edited the summary of this revision. (Show Details)Feb 25 2023, 5:19 AM

In general I like this. Did you see a change in binary size or performance with this change?

libcxx/include/string
1003

Has this change an effect due to the _LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS?

1064–1068

This changes the indention.

philnik marked an inline comment as done.Feb 25 2023, 6:16 AM

In general I like this. Did you see a change in binary size or performance with this change?

I don't have a large code base to check, but here is a small example: https://godbolt.org/z/oYj76b78K (everything is implicitly inline because of constexpr in C++20)

libcxx/include/string
1003

No, I missed these ones in D128081.

In general I like this. Did you see a change in binary size or performance with this change?

I don't have a large code base to check, but here is a small example: https://godbolt.org/z/oYj76b78K (everything is implicitly inline because of constexpr in C++20)

This looks nice :-)

I see the patch failed to apply in the CI, can you fix that?

No other comments, but I like to see a CI run before approving.

libcxx/include/string
1003

Thanks.

philnik updated this revision to Diff 500554.Feb 26 2023, 5:35 AM
philnik marked an inline comment as done.

Rebased

ldionne accepted this revision.Feb 27 2023, 9:55 AM

LGTM w/ green CI.

This revision is now accepted and ready to land.Feb 27 2023, 9:55 AM
This revision was landed with ongoing or failed builds.Mar 19 2023, 9:53 AM
This revision was automatically updated to reflect the committed changes.