[PATCH] D57271: [DebugInfo] Handle restore instructions in LiveDebugValues

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

Thanks, looks generally reasonable!



================
Comment at: lib/CodeGen/LiveDebugValues.cpp:430
+
+  auto ProcessVarLoc = [&MI, &OpenRanges, &Transfers,
+                        &VarLocIDs](VarLoc &VL, MachineInstr *NewDMI) {
----------------
as far as I can this lambda is called three times with the same arguments at the end of each if block. Why can't this code just remain at the end of the function?
Ah.. because VarLoc doesn't yet have an empty constructor?


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:442
+  OpenRanges.erase(VarLocIDs[OldVarID].Var);
+  if (Kind == TransferKind::TransferCopy) {
+    assert(NewReg &&
----------------
I think with three cases, this qualifies for a `switch` statement :-)


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:591
+
+  // To identify a restore instruction, use the same criteria as in AsmPrinter.
+  if (TII->isLoadFromStackSlotPostFE(MI, FI))
----------------
This comment makes it sound as if this should be a method of MachineInstr used by both instead?


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:632
+                                          VarLocMap &VarLocIDs,
+                                          TransferMap &Transfers) {
+  MachineFunction *MF = MI.getMF();
----------------
This is almost the same function as above; how about merging the two for the sake of code size?


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

https://reviews.llvm.org/D57271