[PATCH] D74985: [LiveDebugValues] Encode a location in VarLoc IDs, NFC [2/3]

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:118
+/// for insertion into a \ref VarLocSet, and efficiently converted back. The
+/// type-checker helps ensure that the conversions aren't lossy.
+///
----------------
Should we write this as an adaptor/wrapper of SparseBitVector instead?
I'm fine with "no" as an answer.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:133
+  uint64_t getAsRawInteger() const {
+    return (static_cast<uint64_t>(Location) << 32) | Index;
+  }
----------------
Out of curiosity: This data structure looks like a hand-written implementation of `union { uint64_t ; struct {int32_t; int32_t}}` are there some UB-related problems with doing just that?


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:436
 
-  using VarLocMap = UniqueVector<VarLoc>;
+  class VarLocMap {
+    UniqueVector<VarLoc> VarLocs;
----------------
If it's has methods, it gets a doxygen comment :-)


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:452
     MachineInstr *TransferInst; /// Instruction where this transfer occurs.
-    unsigned LocationID;        /// Location number for the transfer dest.
+    LocIndex LocationID;        /// Location number for the transfer dest.
   };
----------------
Nit: `///<` or the comments must come before the decl.


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

https://reviews.llvm.org/D74985