[PATCH] D69999: [DebugInfo] Support for DW_OP_implicit_pointer (IR Verifier and Bitcode)

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

- Can you update LangRef.rst / SourceLevelDebugging.rst to document the semantics of the DW_OP_implicit_pointer in LLVM IR. Specifically what the two arguments mean while in LLVM IR?
- You'll need to add a Verifier check that rejects DW_OP_implicit_pointer in other positions than the first one.



================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:484
+  /// Returns true if it has implicit pointer expression
+  bool isImplicitPointer() const {
+    return getDebugExpression()->isImplicitPointer();
----------------
I probably wouldn't surface this in MachineInstr at all and just write `getDebugExpression()->isImplicitPointer();` for clarity.


================
Comment at: llvm/include/llvm/CodeGen/MachineInstrBuilder.h:228
+  const MachineInstrBuilder &addMetadata(const MDNode *MD,
+                                         bool IsImplicitPointer = false) const {
     MI->addOperand(*MF, MachineOperand::CreateMetadata(MD));
----------------
I'm not sure this should be exposed here either.


================
Comment at: llvm/include/llvm/IR/IntrinsicInst.h:122
+    /// Returns true if it has an implicit pointer expression
+    bool isImplicitPointer() const {
+      if (!isa<DIExpression>(getRawExpression()))
----------------
Why is this necessary?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:587
+  emitOp(dwarf::DW_OP_implicit_pointer);
+  // this is a dummy offset which will later be replaced with actual offset
+  // of variable in .debug_info
----------------
What does "later" mean in this context?
Also, please make sure to have all comments be full sentences, starting with an uppercase character and ending with a `.`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69999