[llvm] [ADT] Extract DenseMapStorage from DenseMap (NFC) (PR #226664)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Sun Sep 27 00:36:16 PDT 2026
MaskRay wrote:
The generated code is identical (Metadata.cpp and NewGVN.cpp disassemble identically; ScalarEvolution.cpp differs only in the padding bytes covered by one merged zeroing store). But compile time goes up, and the header grows by 36 lines net (+105/−69), mostly forwarding members:
```
┌──────────────────────────────────┬────────────────┐
│ TU │ instructions:u │
├──────────────────────────────────┼────────────────┤
│ lib/IR/Metadata.cpp │ +1.51% │
├──────────────────────────────────┼────────────────┤
│ lib/Transforms/Scalar/NewGVN.cpp │ +1.15% │
├──────────────────────────────────┼────────────────┤
│ lib/Analysis/ScalarEvolution.cpp │ +0.64% │
└──────────────────────────────────┴────────────────┘
```
(clang -O3 -DNDEBUG, no PCH, 4 interleaved rounds, σ ≤ 0.12%)
I guess the cost is the extra class template instantiated per bucket type plus the one-line forwarders the frontend has to instantiate. Does removing CRTP later in the series win this back? If not, I think the refactoring needs a clear benefit to justify both the compile-time cost and the extra lines.
This conflicts with #225018 in DenseMap::allocateBuckets/roundUpNumBuckets and the new grow hook. The conflict is mechanical: the new setStorage/grow move into DenseMapStorage and DenseMap forwards to them. I'd prefer to land #225018 first, since it's a measured win on its own (Metadata.cpp −8% instructions, −21 KB .text), and I'm happy to do the rebase if yours lands first.
https://github.com/llvm/llvm-project/pull/226664
More information about the llvm-commits
mailing list