[PATCH] D70260: [DebugInfo] Support for DW_OP_implicit_pointer (salvagDebugInfo improvement).

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

Hi @alok. I haven't been following the DW_OP_implicit_pointer discussion too closely, so just some inline nitpick comments/questions from me.

Thanks,
Orlando



================
Comment at: llvm/include/llvm/IR/Module.h:197
+  /// we will preserve location of variable in std::map to use later
+  std::map<Value *, Value *> VarAddrMap;
 
----------------
Are you using a `std::map` for a specific reason? Could you not use a `std::unordered_map`, or better yet, a `DenseMap` [0] or similar llvm container here instead?

[0] http://llvm.org/docs/ProgrammersManual.html#llvm-adt-densemap-h


================
Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:1000-1001
 bool DIExpression::extractIfOffset(int64_t &Offset) const {
-  if (getNumElements() == 0) {
+  unsigned numElements;
+  numElements = getNumElements();
+
----------------
Why not `unsigned numElements = getNumElements();` ?


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:1642-1643
   auto wrapMD = [&](Value *V) { return wrapValueInMetadata(Ctx, V); };
+  bool changed = false;
+  Value *debugDeclareAddress = nullptr;
+
----------------
Please can you capitalise these variable names?


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

https://reviews.llvm.org/D70260