[PATCH] D112133: [DebugInfo][Instr] Track subregisters across stack spills/restores

[Jeremy Morse via Phabricator via llvm-commits]

jmorse marked 10 inline comments as done.
jmorse added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:1383
+  // Strictly limit ourselves to plain loads and stores, not all instructions
+  // that can access the stack. Fetch the frame index too.
+  int FI = -1;
----------------
Orlando wrote:
> nit: Is `FI` used anywhere else after these `isXXFromStackSlotPostFE` calls? The comment above makes it sound like the answer is yes, but I can't see it being used anywhere.
Looks like it isn't; it was in the past, I think, now it's un-necessary. I'll rename "DummyFI" and fix the comment.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:1457
+    // subregisters in the destination register line up with positions in the
+    // stack slot.
 
----------------
Orlando wrote:
> Is this an assumption that needs to be chceked in an `assert`, or is it that it's handled gracefully / won't cause a catastrophe if it is ever wrong?
I think we'd have to decode the machine-instruction and work out where it was addressing, to assert that. IMO this is a design assumption: we can't assert that LLVM always does the right thing, only document what was originally expected by this code.

(Another design assumption, for example, is that all modifications to stack slots have a MachineMemoryOperand attached to the instruction, which we can't really assert for).


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

https://reviews.llvm.org/D112133