This is an archive of the discontinued LLVM Phabricator instance.

Add inline to uflow and underflow
AbandonedPublic

Authored by hiraditya on Aug 22 2019, 12:21 PM.

Details

Summary

Called from xsgetn in a loop. Helps with performance.

Diff Detail

Event Timeline

hiraditya created this revision.Aug 22 2019, 12:21 PM
ldionne requested changes to this revision.Aug 22 2019, 1:31 PM
ldionne added inline comments.
libcxx/include/streambuf
284 ↗(On Diff #216687)

What is _LIBCPP_EXTERN_TEMPLATE_INLINE_VISIBILITY? I don't think I've seen it before and I can't seem to find it in the __config header.

This revision now requires changes to proceed.Aug 22 2019, 1:31 PM

The templates basic_streambuf<char> and basic_streambuf<wchar> are externally instantiated in the libc++.dylib.
Marking them as inline would remove them from the dylib, breaking any existing binaries that refer to them.

Is that what you intend?

hiraditya updated this revision to Diff 218591.Sep 3 2019, 10:18 PM

I only intend to inline uflow and underflow.

I only intend to inline uflow and underflow.

I should have written:
Marking uflow and underflow as inline would remove them from the dylib, breaking any existing binaries that refer to them.

Is there a way to have them inlined and still keep the definition in a dylib. I thought inline (with default visibility) will keep the definition in the translation unit.

Normally, inline is used to control the linkage of a function, so that it is ODR-deduplicated across TUs. Correctness of the program is the only thing it should be used for, if we rely on what optimizer folks tell us (e.g. in conferences).

For that reason, my first reaction would be to consider this failed optimization an optimizer bug, not a library bug.

For that reason, my first reaction would be to consider this failed optimization an optimizer bug, not a library bug.

And that, regardless of whether we can make it work as far as symbols-exported-from-the-dylib are concerned.

Normally, inline is used to control the linkage of a function, so that it is ODR-deduplicated across TUs. Correctness of the program is the only thing it should be used for, if we rely on what optimizer folks tell us (e.g. in conferences).

inline hint is used to determine inlining threshold, so it does have effect on llvm's inlining decision. e.g., https://github.com/llvm-mirror/llvm/blob/master/lib/Analysis/InlineCost.cpp#L917

For that reason, my first reaction would be to consider this failed optimization an optimizer bug, not a library bug.

hiraditya planned changes to this revision.Sep 29 2019, 7:34 PM

after collecting numbers

hiraditya abandoned this revision.Oct 13 2019, 4:26 PM