[PATCH] D85760: [DebugInstrRef][7/9] NFC: Separate collection of machine/variable values

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:3043
+  // instructions and recording what machine value their operands refer to.
+  for (unsigned int I = 0; I < OrderToBB.size(); ++I) {
+    MachineBasicBlock &MBB = *OrderToBB[I];
----------------
aprantl wrote:
> can this be `for (MachineBasicBlock &MBB : OrderToBB)`  ?
Yup (ish), updated to use a range based loop.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:3052
+      process(MI);
+      ++CurInst;
+    }
----------------
aprantl wrote:
> What is CurInst used for?
Because I'm identifying each value by the instruction defining it, each instruction needs a unique number which I derive from its block number and position within that block. See the ValueIDNum class in D83047, every value number is a tuple {block-number, instruction-number, register-or-spill-number}. In this loop we have to step through those numbers, with the "current" block / instruction number in CurBB / CurInst.

I guess this is easily replaceable by passing those numbers to "process" and its helpers rather than storing it in the object, and that would closer match what the VarLoc LiveDebugValues does. I'll add this to a list of things to polish, unless it's important for this patch.


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

https://reviews.llvm.org/D85760