[PATCH] D58042: [LiveDebugValues] Emit parameter's entry value

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:215
+        return Other.Kind < Kind;
+      return Loc.Hash < Other.Loc.Hash;
     }
----------------
FYI, I recently learned a neat trick:

`return std::tie(Var, Kind, Hash) < std::tie(Other.Var, Other.Kind, Other.Hash);`

IIUC, this is equivalent to the above code?


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:219
 
+  using ParamSet = SmallVector<MachineInstr *, 6>;
   using VarLocMap = UniqueVector<VarLoc>;
----------------
How was 6 derived (I'm asking because a non-power-of 2 always looks very deliberate).


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:298
+                       VarLocMap &VarLocIDs, TransferMap &Transfers,
+                       ParamSet &ParamEntryVals,
+                       SparseBitVector<> &KillSet);
----------------
If you don't want the vector size to leak into the function, you can use a `SmallVectorImpl<MachineInstr *>`


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:431
+  // (with/out entry value) as location.
+  VarLoc::VarLocKind K = VarLoc::InvalidKind;
   if (isDbgValueDescribedByReg(MI)) {
----------------
`K` -> `Kind`


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:598
+
+  auto *TPC = getAnalysisIfAvailable<TargetPassConfig>();
+  auto &TM = TPC->getTM<TargetMachine>();
----------------
`IfAvailable` ... can this return a nullptr?


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:935
+  auto IsRegOtherThanSP = [&](const MachineOperand &Op) -> bool {
+    return Op.isReg() && Op.getReg() && Op.getReg() != SP;
+  };
----------------
I think the middle condition is redundant? Or do we expect SP to be 0?


================
Comment at: lib/CodeGen/LiveDebugValues.cpp:952
+  auto IsNewParameter = [&ParamEntryVals](const MachineInstr &MI) {
+    for (auto MII : ParamEntryVals)
+      if (MII->getDebugVariable() == MI.getDebugVariable())
----------------
`return std::any_of()`


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

https://reviews.llvm.org/D58042