[PATCH] D114394: Compile-time computation of string attribute hashes

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

Compile-time: https://llvm-compile-time-tracker.com/compare.php?from=3608e18a946e77a474a468304b6c3904c55dbce0&to=0ce74a09cc8533a10fb67fdec4fb6ad8de4f1153&stat=instructions Some improvement at `O3`, not much change for optimized builds.

A concern I have is that this may be pessimizing non-clang frontends in favor of clang. Frontends not written in C++ will always be going through the `AttributeKey::get()` API, which will be slower than the initial state (I think -- we still have to calculate the hash dynamically, but now we also need to intern an AttributeKey by performing an actual hash table lookup.)

As a more general thought, I believe a core (performance) problem in the current attribute design is that AttrBuilder represents string attributes as a pair of strings, and most of our attribute manipulation is indirected through AttrBuilders. This means that we end up converting string `Attribute`s into a pair of strings and then construct an `Attribute` again via FoldingSet lookup. This is of course a lot of unnecessary work. Similarly, there are many hot attributes (like `"target-cpu"`) which are basically always the same, and we could just construct the `Attribute` upfront and always use that. That would save more than just the attribute name hash calculation. I think we may want to reconsider the AttrBuilder design to store `Attribute`s at least for string attributes, though of course that does make it Context-dependent.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114394/new/

https://reviews.llvm.org/D114394