[PATCH] D49560: Add support to track total counts for DebugCounter

[Zhizhou Yang via Phabricator via llvm-commits]

zhizhouy added inline comments.


================
Comment at: include/llvm/Support/DebugCounter.h:162
+  };
+  DenseMap<unsigned, CounterInfo> Counters;
   DenseMap<unsigned, std::string> CounterDesc;
----------------
george.burgess.iv wrote:
> zhizhouy wrote:
> > george.burgess.iv wrote:
> > > I don't know the history of this code, but it seems reasonable to me to make this a `vector`, add a `string Desc` to `CounterInfo`, and drop `CounterDesc` entirely. `CounterVector` just returns `[1..UINT_MAX]`, in order, on insertion anyway.
> > > 
> > > (Alternatively, it seems that we could replace all three of these containers with a single `StringMap<CounterInfo>` and hand `CounterInfo *`s out instead of `unsigned`s, but that starts to stray pretty far from the goal of this patch, so... :) )
> > I have moved Desc into the CounterInfo struct. Using vector to replace DenseMap may cause some ambiguity from UniqueVector on index. So I am keeping the DenseMap for now. We may come up with another patch to do the second change you mentioned.
> This WFM, but I don't quite understand what you mean by
> 
> > Using vector to replace DenseMap may cause some ambiguity from UniqueVector on index
> 
> For my curiosity, can you elaborate a bit, please? :)
So in DebugCounter::print() and getCounterInfo(), ID for RegisteredCounters and Counters comes up together. So no matter we return ID or ID-1, we need to +/- 1 or one of them.


Repository:
  rL LLVM

https://reviews.llvm.org/D49560