Add _LIBCPP_PLACEMENT_NEW_DEFINED macro to allow for user defined placement new operators
#70538
Conversation
…DEFINED` macro to allow for user defined placement new
|
I'm going to go with "just include |
I just looked and our I am quite reluctant to add any kind of complexity to the library to work around something like this -- I don't think that's the right tradeoff here. |
|
Reopening, I think there's something I don't understand here.
If you I don't think it is worth doing something about the "compilation time issue", however if we do something that pessimizes how Clang handles our stdlib-declared placement new, then it is probably reasonable to do something about that. |
|
Clang always treats placement-new as an intrinsic, so I think we can close this: https://godbolt.org/z/fPjKja3cK |
Since clang always treats the reserved placement-new as an intrinsic, even going back to clang 3.x, can we remove The definition would go from to this |
https://godbolt.org/z/cT4GK3MbY for reference.
Background
I work on a large software project where compile-times and debug performance is a priority.
We also have our own stl equivalent, for the most part, that also implements some of the low-level language bits that are intended to be provided by the
std.Usage of
stdis near non-existent except when interfacing with certain vendor or third-party libs.On the platforms we ship on there are a variety of
stdimplementations includinglibcxx.Issue
Clang treats the reserved placement new as an intrinsic as long as it is declared within the TU.
https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGExprCXX.cpp#L1587 for reference.
This avoids generation of a function call and avoids needing to include
<new>to ensure what effectively should be a language feature in C++.Since
libc++adds anabi_tagattribute this means if<new>is included after our declaration, such as from a vendor or third-party header when interfacing with those APIs, we get a compile-time error.To cover our bases in these rare cases means we have to forego using the placement-new intrinsic in the majority of cases on platforms where
libcxxis a potentialstdimplementation.If code includes
<new>in a TU after our declaration we know we are not getting the compile-time benefits. This case is a small number of cpp files where we have to interface with vendor libraries and need to include vendor headers unlike the majority uses of placement new which is ubiquitous in C++.Also the removal of a linker symbol and debugging information is an added bonus when using the intrinsic.
MSVC stl does provide a
__PLACEMENT_NEW_INLINEmacro to allow the user to provide their own placement new operators as a point of reference.Answers to potentially common questions
Just include
<new>and stop working around thestdInclude times.
<new>is expensive to include on a variety ofstdimplementations.Use forceinline on clang with a custom placement new
The debug code generated is worse than the intrinsic as shown in the godbolt above.
It's just one function call. Deal with it.
Debug performance is death by 1000 cuts. In isolation one function call isn't a big deal.
Across a large project it can be a major perf hit especially small functions like
std::move.Plus what often goes unnoticed is the extra stack usage in debug and extra function calls do not help there when trying to avoid stack overflows.
PCH and/or unity builds
We do not use unity builds.
We have to disable PCH for some compilers due to bugs usually around builtin usage within a header.
If you work on the headers in a dev branch included in the PCH then you are just rebuilding every source file.
Depending on the team/engineer it may be common to see PCH builds for a module disabled locally for iteration.
Plus not every source file benefits from PCH depending on the includes within the PCH and what headers that source file actually uses.
Alternate Solution
An alternate solution is to internally declare and define the reserve placement new operators within clang itself.
That way they are available in any TU without needing
<new>from astdimplementation.Personally I believe most of the constructs in the
stdshould be language level constructs.If this is a viable alternate solution let me know and I can submit a PR to clang itself.
We could also remove
_LIBCPP_INLINE_VISIBILITYif we are compiling under clang since we can assume clang will always treat this reserved operator as an intrinsic and thus never emit a function call.Let me know what you prefer :).