[PATCH] D59941: [DebugInfo] Improve handling of clobbered fragments

[David Stenberg via Phabricator via llvm-commits]

dstenb marked an inline comment as done.
dstenb added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp:111
+// pointers.
+using DbgValueEntriesMap = std::map<InlinedEntity, SmallSet<EntryIndex, 1>>;
+
----------------
dblaikie wrote:
> aprantl wrote:
> > dstenb wrote:
> > > aprantl wrote:
> > > > Do we really need the properties of a std::map here and in the line above?
> > > > So far every time I tried using a std::map in AsmPrinter a std::vector + std::sort turned out to be faster and to use less memory.
> > > There is at least be no need for iterators to be valid after insertion/deletion, nor is the iteration order important for determinism, which I guess is the two of the main benefits of std::map compared to LLVM's own map implementations. I tried switching both to DenseMap, but I did at least not see any effect on the build time when building the RelWithDebInfo+asan clang binary, but I'm not sure what conclusions I should draw from that.
> > > 
> > > I will look into switching both to sorted vectors.
> > ... or sorted llvm::SmallVectors.
> Usually the issue for this tradeoff (map/set (be it standard or LLVM's custom data structures) versus sorted vector (be it std::vector or SmallVector)) is whether or not there's a separate "gather" and "lookup" phase.
> 
> If lookups are required while the data structure is also growing, then it's probably inefficient to keep resorting the data structure to keep those lookups efficient.
> 
> if there's a defined point where all building is done (& no querying - so you probably know/can guarantee you aren't inserting duplicates without having to test), a single sort and then purely lookup from that point on.
> 
> Is that the case here?
I don't think this is the case here. There is not really separate "gather" and "lookup" phases for the maps, and instead the insertion and querying is interleaved. Both maps may grow/shrink when encountering new debug values or clobbering instructions throughout the iteration of the machine function, so there is not really a defined point where we know we are done building the maps.


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

https://reviews.llvm.org/D59941