[PATCH] D140758: [llvm][AsmPrinter][NFC] Cleanup `GCMetadataPrinters` field

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 29 05:02:40 PST 2022


dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

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.



================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:3822-3834
+  if (auto GCPI = GCMetadataPrinters.find(&S); GCPI != GCMetadataPrinters.end())
     return GCPI->second.get();
 
   auto Name = S.getName();
 
   for (const GCMetadataPrinterRegistry::entry &GCMetaPrinter :
        GCMetadataPrinterRegistry::entries())
----------------
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)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140758



More information about the llvm-commits mailing list