[PATCH] D111317: [DebugInfo][InstrRef] Track instructions that write-to-stack after having a spill fused into them

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

Orlando added a comment.

LGTM, though I'm not sure I feel qualified to press the green button myself in this instance. I left a few inline nits, plus please make the clang-format bot happy.



================
Comment at: llvm/include/llvm/CodeGen/MachineFunction.h:549
+  /// for instructions that have a stack spill fused into them.
+  static unsigned int DebugOperandMemNumber;
+
----------------
Can you use const/constexpr here so that you can include the definition here in the header? Or is there some reason that this isn't const?


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:1452-1457
+  // Strictly limit ourselves to plain loads and stores, not all instructions
+  // that can access the stack.
+  int FIDummy;
+  if (!TII->isStoreToStackSlotPostFE(MI, FIDummy) &&
+      !TII->isLoadFromStackSlotPostFE(MI, FIDummy))
+    return false;
----------------
This is repeated a few lines above, between 1442-1445 (patch juggling?)


================
Comment at: llvm/test/DebugInfo/MIR/InstrRef/memory-operand-tracking.mir:39
+debugValueSubstitutions:
+- { srcinst: 2, srcop: 0, dstinst: 3, dstop: 1000000, subreg: 0 }
+stack:
----------------
Is there a way to make the IR printer/reader to print/read `MemOperand` or something instead of `1000000` (if yes and you agree - this could be a separate patch)?


================
Comment at: llvm/test/DebugInfo/MIR/InstrRef/memory-operand-tracking.mir:54-57
+    ;; We should _not_ be able to find the old value.
+    DBG_INSTR_REF 1, 0, !11, !DIExpression(), debug-location !12
+    ; CHECK:      DBG_INSTR_REF 1, 0
+    ; CHECK-NEXT: DBG_VALUE $noreg
----------------
What is the purpose of this part of the test?


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

https://reviews.llvm.org/D111317