The field is currently void*, which was originlly chosen in 2010 to not need to include DenseMap. Since then, DenseMap has been included in the header file anyways, so there is no more need to for the indirection via void* and the cruft around it can be removed.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Comment Actions
Sounds good - if you've got any links to the previous conversation that mentions the justification of using void*, might be good to link to those references in the commit message for completeness.
| llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | ||
|---|---|---|
| 3822–3835 | This could be modified (in a separate change) to avoid doing two lookups (one find, one insert) up GCMetadataPrinters in the cache miss case by doing something like this: auto [GCPI, Inserted] = GCMetadataPrinters.insert({&S, nullptr});
if (!Inserted)
return GCPI->second.get();
auto Name = S.getName();
for (...) {
if (...) {
...
GCPI->second = std::move(GMP);
return GCPI->second.get();
}
}(assuming none of the operations (like GCMetaPrinter.instantiate()) cause other modifications to the map that would invalidate the iterator between the current find and insert calls) | |
Comment Actions
I thought I had found a more elaborate discussion back when I actually wrote this change, but right now I can only find the original commit that explicitly did that change (https://github.com/llvm/llvm-project/commit/2cf5f9ec05d62dac79c0df240e0b847d3658b084).
Not explicitly said, but its the only intention I can infer from the commit description
| llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp | ||
|---|---|---|
| 3822–3835 | Thought it was a minor change so updated the diff to include it as well | |
This could be modified (in a separate change) to avoid doing two lookups (one find, one insert) up GCMetadataPrinters in the cache miss case by doing something like this:
auto [GCPI, Inserted] = GCMetadataPrinters.insert({&S, nullptr}); if (!Inserted) return GCPI->second.get(); auto Name = S.getName(); for (...) { if (...) { ... GCPI->second = std::move(GMP); return GCPI->second.get(); } }(assuming none of the operations (like GCMetaPrinter.instantiate()) cause other modifications to the map that would invalidate the iterator between the current find and insert calls)