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

2021-12-15 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. Update: the strategy used by ``AttrBuilder`` might not be the best way to build new attributes. I've proposed an alternative implementation in https://reviews.llvm.org/D115798. This is somehow orthogonal to that patch, but it removes the need for temporary cop

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

2021-12-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. That's interesting, somehow clang spends 1% of its time for sqlite3 in attribute lookup. I wonder how we ended up doing so much string-based attribute lookup. These were not really ever intended to be used for anything semantically interesting for the middle end, they were

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

2021-12-09 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 393058. serge-sans-paille added a comment. Use a DenseSet instead of a DenseMap to store the StringPool. Some benchmark feedback (using a release build compiled with LTO), in number of instructions | | sqlite3.c -c -O0 | sqlite3.c -S

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

2021-12-07 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. *comment removed, I've been doing more detailed benchmark that imply a rework of the patch* CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114394/new/ https://reviews.llvm.org/D114394 ___ cfe-commits mailing

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

2021-12-07 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. > 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

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

2021-11-30 Thread 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

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

2021-11-30 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment. Gentle ping :-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114394/new/ https://reviews.llvm.org/D114394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listi

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

2021-11-24 Thread serge via Phabricator via cfe-commits
serge-sans-paille marked an inline comment as done. serge-sans-paille added inline comments. Comment at: llvm/include/llvm/IR/Attributes.h:79 + bool operator<(AttributeKey const &other) const { +return strcmp(value_, other.value_) < 0; + } JonChesterfield w

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

2021-11-24 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 389463. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114394/new/ https://reviews.llvm.org/D114394 Files: clang/lib/CodeGen/CGCall.cpp clang/lib/CodeGen/CGOpenMPRuntime.cpp clang/lib/CodeGen/CodeGenFunction.cpp llvm/include/llvm/Ana

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

2021-11-24 Thread Yilong Guo via Phabricator via cfe-commits
Nuu added inline comments. Comment at: llvm/include/llvm/IR/Attributes.h:54 +size_(HashedS.size()) { +assert(hash_ == hasher(s.data(), s.size()) && "consistent hashing"); + } CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114394/new/ h

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

2021-11-24 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 389437. serge-sans-paille marked 6 inline comments as done. serge-sans-paille added a comment. Use CachedHashStringRef as suggested by @MaskRay CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114394/new/ https://reviews.llvm.org/D114394 Fil

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

2021-11-23 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added inline comments. Comment at: llvm/lib/IR/Attributes.cpp:125 FoldingSetNodeID ID; - ID.AddString(Kind); + ID.AddString(Kind.value()); if (!Val.empty()) ID.AddString(Val); Carrying over my comment from the original revision: it seem that y

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

2021-11-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: llvm/lib/IR/AttributeImpl.h:214 - DenseMap StringAttrs; + std::unordered_map StringAttrs; std::unordered_map is inefficient. Why is this change? Comment at: llvm/lib/IR/Attributes.cpp:862 + auto

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

2021-11-23 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added subscribers: aeubanks, rnk. rnk added a comment. I think @aeubanks might be a good reviewer for this. I like the performance wins here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114394/new/ https://reviews.llvm.org/D114394 ___

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

2021-11-23 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 389187. serge-sans-paille added a comment. Remove static Dict and replace it by a dict attached to LLVMContext. Some early benchmarks, on the SQLite amalgamation, through ` valgrind --tool=callgrind ./bin/clang -c -o/dev/null sqlite3.c` Instructio

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

2021-11-23 Thread Jon Chesterfield via Phabricator via cfe-commits
JonChesterfield added inline comments. Comment at: llvm/include/llvm/IR/Attributes.h:79 + bool operator<(AttributeKey const &other) const { +return strcmp(value_, other.value_) < 0; + } Could order by size first here, then strncmp on equal sizes Repositor

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

2021-11-22 Thread serge via Phabricator via cfe-commits
serge-sans-paille created this revision. serge-sans-paille added reviewers: dblaikie, nikic. Herald added subscribers: ormris, dexonsmith, wenlei, okura, jdoerfert, kuter, haicheng, hiraditya, eraman. serge-sans-paille requested review of this revision. Herald added a reviewer: jdoerfert. Herald a