[PATCH] D84115: [Debuginfo][Codegen] (2/7) Support for DW_OP_implicit_pointer for named and unnamed variables (second strategy).

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2715
+    for (unsigned i = 0; i < getNumElements(); ++i) {
+      if (Elements[i] == dwarf::DW_OP_LLVM_implicit_pointer)
+        Count++;
----------------
early continue?


================
Comment at: llvm/include/llvm/IR/IntrinsicInst.h:163
+
+  /// Returns true if it has an explicit pointer expression.
+  unsigned implicitPointerCount() const {
----------------
true/false but the return val is unsigned?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DebugLocEntry.h:53
 
-  /// Either a constant,
+  /// a constant,
   union {
----------------
intentional change?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h:142
+
+    return (long unsigned)std::distance(ImplicitVars.begin(), It);
+  }
----------------
`static_cast` might be better


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1835
+    // In case of implicit pointer, implicit target can not initialize
+    // variable so we need to create location list even in case of single value
+    if (!MInsn->getDebugExpression()->isImplicitPointer() &&
----------------
missing `.` at the end


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2469
+        // all we have is index in a list of variable, we can now compute
+        // actual offset
+        unsigned ValOffset = 0;
----------------
same here


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2471
+        unsigned ValOffset = 0;
+        // read the dummy offset (index)
+        for (uint64_t J = Op.getOperandEndOffset(I) - 1; J >= Offset; --J)
----------------
please refactor these comments


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

https://reviews.llvm.org/D84115