[PATCH] D68209: [LiveDebugValues] Introduce entry values of unmodified params

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

Hi @djtodoro, thanks for the patch as always. I'm curious about the `DbgEntryValueMovement` field. Could you explain it in a little more detail? At a high level, IIUC, it's used to mark copies of entry values for the purpose of simplifying location lists (i.e. keep an original AT_entry_value location description even if there are copies). Is that correct? Does the approach handle there being multiple copies of an entry value within a block?



================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:727
+  const MachineInstr *DebugEntryValue =
+      const_cast<MachineInstr *>(DebugEntryVals[Var].DbgEntryValue);
+  assert(DebugEntryValue->getOperand(0).isReg() &&
----------------
Why is the const_cast needed?


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:831
+                      .getReg())
+            : VarLoc::CreateEntryLoc(*ParamDebugInstr, LS, NewExpr);
 
----------------
It would be good to simplify this expression, possibly by sinking it into a helper function that returns a VarLoc.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:1122
+  if (!DestRegOp->isDef())
+    return;
+
----------------
Why move the DestRegOp->isDef() check to after the `isCalleSavedReg` lambda definition?


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:1152
+  if (isRegOtherThanSPAndFP(*DestRegOp, MI, TRI)) {
+    for (auto &EnltyValue : DebugEntryVals) {
+      if (isCopyOfTheEntryValue(EnltyValue.second)) {
----------------
`auto &EntryValue`?


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

https://reviews.llvm.org/D68209