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

[Alok Kumar Sharma via Phabricator via llvm-commits]

alok marked 5 inline comments as done.
alok added inline comments.


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


================
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));
----------------
aprantl wrote:
> I'm not sure this should be exposed here either.
Other option is to duplicate this function for implicit pointer only difference being the absence of first assert. Please let me know your suggestion?


================
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()))
----------------
aprantl wrote:
> Why is this necessary?
It has call from other functions. Please let me know if i should inline the content at call-sites?


================
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
----------------
aprantl wrote:
> 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 `.`.
By later i meant function DwarfDebug::emitDebugLocEntry.
I shall update it in next revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69999