[PATCH] D137096: [llvm-readobj] Provide Hash Histogram for all ELFDumper implementations

[Paul Kirth via Phabricator via llvm-commits]

paulkirth marked 8 inline comments as done.
paulkirth added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:7130-7132
+void LLVMELFDumper<ELFT>::printHashHistogramStats(
+    size_t NBucket, size_t MaxChain, size_t TotalSyms,
+    const ArrayRef<size_t> Count) const {
----------------
jhenderson wrote:
> Any particular reason you've made the `Count` member `const` in these two methods?
Thanks for pointing that out. I think it was just habit from writing `const Foo&` forever, but that doesn't make sense in this case. 


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:7155
+  size_t CumulativeNonZero = 0;
+  for (size_t I = 0; I < MaxChain; I++) {
+    CumulativeNonZero += Count[I] * I;
----------------
jhenderson wrote:
> https://llvm.org/docs/CodingStandards.html#prefer-preincrement
This loop is taken directly from the `GNUELFDumper`, should I update this in the existing implementations too? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137096