[PATCH] D112333: [DebugInfo][InstrRef][NFC] Use DenseMaps where we can, use more efficient ValueIDNum repr

Orlando Cazalet-Hyams via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 25 05:35:36 PDT 2021


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

LGTM - please clang-format.

> Strangest of all: replacing the std::map in TransferTracker::loadInLocs with a DenseMap causes a performance loss

Some maybe-useless speculation: Perhaps the current `ValueIDNum` hash is poor / causing high number of collisions? `std::map` (as opposed to `DenseMap` and `std::unorderd_map`) is ordered. Maybe in this case, the ordered search is quicker than a hash table lookup or something like that?



================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h:119
+
+  static_assert(sizeof(u) == 8, "Badly backed ValueIDNum?");
 
----------------
backed -> packed?


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h:1029
+  static unsigned getHashValue(const ValueIDNum &Val) {
+    return Val.asU64();
+  }
----------------
Curious if you have you tried experimenting with this hash function at all?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112333



More information about the llvm-commits mailing list