[clang] Add clang::behaves_like_std(...) attribute
#76596
Conversation
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Max Winkler (MaxEW707) ChangesBackgroundhttps://godbolt.org/z/hv53svTrq for reference on all of the below. In games debug performance is critical as much as optimized performance. However us and many other game studios do not use the Use CaseWe have our own core libraries and STL implementation. ClangCLLast year MSVC added Besides the overloaded use of the word Force Inline MoveTo get around the GCC/Clang we do currently mark our move/forward functions as force inline which is respected in debug. Just use stdWhile we might be able to use MSVC STL shipped with MSVC 1938 takes 45ms to parse with clang. We cannot include such costly headers into basically every single source file. We also don't want to balloon our PCH headers with expensive std includes and instead keep them for our internal includes as much as possible since the larger the PCH the slower your builds. Just declare moveClang only looks for the declaration so we can just declare We still need to ensure that our headers work with
|
There was a problem hiding this comment.
While I disagree with the reasoning for not just using std (@MaxEW707 I think having a chat about this informally would be nice.), I do think it makes sense to add the ability to declare some functions as builtins. There are a few cases inside the standard library itself:
- simply allowing the same optimizations to be done with
-ffreestanding - there are quite a few more functions which behave just like builtins, but are currently not recognized as such by the compiler. e.g.
identity::operator() - library-internal functionality which would have to be handled by the compiler for every implementation, like
__identity::operator()in libc++
The above ones are all handled by what is currently proposed, but there is also functionality which doesn't work with this proposal. For example:
- allowing the declaration of non-
stdbuiltins, likechar_traits<char>::find(), which could be declared as equivalent to__builtin_memchr. - other functions which are essentially casts, but take their arguments by-value, like
to_underlying - Functions that are essentially builtin operations, like
char_traits<char>::eq
I don't know whether all of these things should be handled with the same attribute, or whether they should be handled at all, but they should definitely be considered when designing this attribute.
| template <class T> struct remove_reference { typedef T type; }; | ||
| template <class T> struct remove_reference<T&> { typedef T type; }; | ||
| template <class T> struct remove_reference<T&&> { typedef T type; }; |
There was a problem hiding this comment.
Maybe just use __remove_reference_t?
Sounds good. Let me know the best way to contact you.
I'll try to give more context here. The main reason is include times and not wanting to work around a variety of vendor STLs who have wildly different include times and behaviours. On one of our smaller projects the PC build is ~16,000 cpp files which includes the generated cpp files, the tools, engine, tests, and the game itself. This is the build that most engineers work on and they usually need to work across a variety of domains since a change in the engine may require a change in the code generator, editor, asset building, the unit tests, the integration tests, etc. On this smaller project one of our console builds which only includes the game itself is around ~4k source files when I last checked. Just adding We do not use the STL anywhere in our code base. We have our own so just including any STL header also brings in other headers like At the end of the day we don't want to deal with different STL implementations. |
For this I am more concerned with language level features such as move, forward, reserved placement operator new, etc and not all generic std functions. I can see where that becomes muddy. For example I would love for I would also love for I foresee the compiler increasing making assumptions about the names of functions and types inside the std namespace like There are cases where the std functions have other implicit properties. Reserved placement operator new can be assumed to not have to check the returned pointer for null. If you implement your own placement operator new you do not get this benefit. MSVC only removes this check for the reserved placement new but thankfully provides the define Maybe for stuff like I haven't thought too much about those other cases yet but if |
|
I struggle to understand the motivation here: If you are not using a standard library implementation at all and instead act as your own standard library vendor, just providing a declaration of move / forward should be enough. The concern about ABI tags only come up if the STL is indeed included, and you seem to say it never is. Can you explain a bit more why you are concerned about how things are defined in libc++? @philnik777 Do I prefer this approach over More general observations on the "using c++ without the STL thing": That being said, I understand the pain. I think we could also extend the set of std functions replaced in the front end if we can show it would have measurable compile times improvements (ie to_integer, as_const, to_underlying, etc). PS: I appreciate that this patch comes with complete documentation :) |
If that were always the case we could simply provide a declaration without ever defining the function, but clang and gcc don't always replace it. e.g. if you take the address (which both compilers unfortunately still allow). While it's unlikely for |
|
I appreciate the discussions. I know I have a tendency to get ranty in my writing so I just want to be clear that I mean no malice.
I don't know how else to make it clear that we will never be including std headers from our core library headers and within the core engine itself. The include times are a non-starter. If the verdict is just use std, we are going to continue just marking these functions as Even if libc++ becomes tolerable we have to deal with msvc stl, libstdc++, libc++, SCE stl, and vendor forks of libc++ that I will not mention since this specific company is super duper litigious over the most minor things all compiled under clang. I also want to highlight that we in particular have a wide range of platform support. On our Linux servers we have such a variety of distros and GCC/Clang versions. Regarding modules. Try moving the titanic that is [insert favourite game here that is shipping content every year]. Apple Clang + Xcode still don't support modules last I checked. We haven't upgraded to Xcode 15 yet. We have our own build system generator. We have our own package management system. We have our stuff working well with PCH files where needed.
I don't want to speak for my peers but I think I can speak to the general sentiment of my community. I have followed the Reflection TS and its fine. At every game company I worked at our reflection needs are so esoteric that we are going to continue writing our own with our own code generators. As mentioned above our Linux servers use a variety of distros. The server software running in our datacentres still uses all our core libraries like all the game software but has more liberty with what game teams can run. We are less strict on what the various amount of server services are allowed to use since that software is usually tightly written for one specific platform. For example I know, https://github.com/confluentinc/librdkafka, is a common library used in some backend services across the games industry which has a C++ interface with std. I've seen kafka used for log aggregation, game server monitoring, login queue monitoring, etc in backend services.
I first want to talk about freestanding. We cannot ship with freestanding on consoles and mobile. As I tried to explain above we do not use the std but we are forced to use it in some cases. It is very likely that std headers will get included after our headers transitively in platform specific files. For example since it is super dead now, Stadia. All of Stadia's APIs had std in them. A lot of game studios use https://github.com/syoyo/tinyexr to handle OpenEXR image formats when converting from RAW assets to Editor assets and/or Editor assets to Runtime assets. There are many more examples especially vendor code from some platforms that aren't public that we cannot at all control. Our style guide requires system/library headers to be included after our headers and I don't even want to start the bikeshedding on changing that...
As @philnik777 highlighted the std implementers have to implement it pedantically and guard against users doing insane things like taking the addressof
Maybe an attribute name change would suffice here. Something like The function could be something crazy like or as simple as The warnings not working with this proposed attribute is not a big deal to me. We already don't get them with our implementation of these meta functions. I intend to fix these warnings anyways and I have a PR already for one here: #76646. static assertions possibly not firing such as Maybe this is an easy solve with the proposed I think I addressed all of the statements above. Please let me know if I missed any :). |
|
It's not ready, but I have a draft for a much more general attribute: #78071 |
I was playing around with your draft PR and I do prefer your solution over mine. Let me know if you want me to close this PR in favour of your PR. |
|
Adding @AaronBallman and @erichkeane for a wider audience |
|
One question I have is related to:
Clang supports attributes in the |
Background
https://godbolt.org/z/hv53svTrq for reference on all of the below.
In games debug performance is critical as much as optimized performance.
We mainly accomplish this by reducing the amount of function calls being made in debug builds. This does not imply the use of
always_inlinebut mainly being diligent in the amount of small functions being called such as getters and accessor functions.As has been gaining support the last couple years and shown by clangs builtin support there are a lot of language level features that are implemented in the standard library such as
std::movewhich lead to poor debugging performance, experience and extra compile time due to template instantiations.Since clang already treats these meta cast functions as implicit builtins I will assume the reasoning for such is already a given.
However us and many other game studios do not use the
stdand have our own core libraries and/or STL implementations. Therefore we do not get the benefits that clang provides by recognizing certain functions inside thestdnamespace. I will try to list some reasons why we cannot just include<utility>and use the std functions. I am happy to expand on those reasons in the comments if desired. The EASTL paper here is a good starting point and still relevant today in my opinion.Use Case
We have our own core libraries and STL implementation.
Internally our STL and core libraries use
static_castto implement move and forward semantics.However almost all user code, the engine and games themselves, use the provided move and forward template functions from these libraries.
Currently we have two variants due to historical reasons that will most likely never converge. One is inside a namespace like so
MySTL::move(...)and one is prefixed as is done in C code like soMyMove(...).ClangCL
Last year MSVC added
[[msvc::intrinsic]]for us game devs here. This was explicitly added as an attribute under the request of us since a lot of us have custom STLs.Clang currently lacks such an attribute which means we can't fully move onto
ClangCLuntil we have a similar facility. It would also be helpful to have this attribute for our other platforms which are mostly console vendors and mobile who supply their own fork of clang but more on that below.Besides the overloaded use of the word
intrinsicone downside to this approach is that any warnings that could be generated by knowledge ofstdfunction implicit behaviours are not generated. Pessimizing move return warnings aren't generated.This is still the case for our explicit core code that uses
static_castand something I plan to improve within Clang in the future.Force Inline Move
To get around GCC/Clang we do currently mark our move/forward functions as force inline which is respected in debug.
However as the godbolt shows the codegen isn't as good as the builtin in debug.
We still incur the compile-time cost compared to the builtin.
We do not use force inline liberally and have disabled this in past for some compilers where we noticed force inline regressions in compile times. We haven't noticed such regressions lately.
Just use std
While we might be able to use
std::movevia using declarations and macros that expand tostd::movewe cannot at all suffer the include times from vendor std implementations. Do note that we need to ship across different STL versions and vendor STLs that are not one of the major 3 public STLs, as referenced below, that we all know.MSVC STL
<utility>shipped with MSVC 1938 takes 45ms to parse with clang.libstdc++
<utility>shipped with Ubuntu 22.04 takes 31ms to parse with clang.libc++
<utility>shipped with Ubuntu 22.04 takes 156ms to parse with clang.We cannot include such costly headers into basically every single source file. We also don't want to balloon our PCH headers with expensive std includes and instead keep them for our internal includes as much as possible since the larger the PCH the slower your builds.
I haven't looked into
-fpch-instantiate-templatesyet or whether MSVC/Ycstill has the same behaviour as-fpch-instantiate-templatesto deal with PCH performance issues. As MSVC gets more conforming and since we have to deal with vendor clang forks and vendor libraries, some which include templates in their headers, I don't want to be enforcing non-conforming defaults on our game teams using our core libraries.Just declare move
Clang only looks for the declaration so we can just declare
moveinside thestdnamespace.We have done this in the past however
libc++started marking a bunch of functions withabi_tagwhich cannot be redeclared as shown in the godbolt.We still need to ensure that our headers work with
stdheaders included after us because there are some platform specific source files that need to interact with vendor or third party libraries who do use and includestdfrom their headers.clang::behaves_like_stdIntroducing
behaves_like_stdattribute which can be used to mark a function that has the same guarantees as thestdequivalent.behaves_like_std("move")will notify clang that your function has the same semantics asstd::moveand should be treated as such.This also has the benefit of still emitting all the
stdwarnings such as self move and pessimizing move.In the future we can probably extend this attribute to support more
stdbehaviours especially as the compiler is increasing being asked to poke into thestdnamespace in every C++ standard and as the C++ std gets more meta functions that are implemented in the library but not the language itself.I don't mean for any of this to come off as a rant. Trying to plead my case for the viability of this attribute and its necessity for some users of C++ which in my case is the games industry.