[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