[PATCH] D50887: [DWARF] Missing location debug information with -O2.

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

This is looking really good. I think it's great that the test exercises more than just MachineCSE in isolation. Just two more minor comments (inline) --



================
Comment at: include/llvm/CodeGen/MachineInstr.h:1528
+  /// Scan instructions following MI and collect any matching DBG_VALUEs.
+  void collectDebugValues(MachineRegisterInfo *MRI,
+                          SmallVectorImpl<MachineInstr *> &DbgValues);
----------------
This function won't work as intended if `MRI` is null. Could you make it a reference so that future developers know to pass in a valid object?


================
Comment at: lib/CodeGen/MachineInstr.cpp:2037
+                                SmallVectorImpl<MachineInstr *> &DbgValues) {
+  DbgValues.clear();
+  const MachineOperand &MO = getOperand(0);
----------------
While we're improving this utility, wdyt of removing this `clear`? It doesn't seem to be necessary.


https://reviews.llvm.org/D50887