[PATCH] D80684: [LiveDebugValues] Speed up removeEntryValue, NFC

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:917
       const VarLoc &VL = VarLocIDs[LocIndex::fromRawInteger(ID)];
-      if (!VL.isEntryBackupLoc())
-        continue;
----------------
vsk wrote:
> djtodoro wrote:
> > I'm not sure we want to remove this line. The backup locations are `EntryValueBackupKind` and `EntryValueCopyBackupKind` only; the `EntryValueKind` is real location created from one of these two.
> It looks like `VL.getEntryValueCopyBackupReg()` returns undef when VL.Kind isn't the right kind. So it seems safe to remove the `VL.isEntryBackupLoc()` check?
Oh, I see. It is safe! Thanks.
Eventually, if we get rid of the [1] we won't need this part at all. The `EntryValueCopyBackupKind` was used to indicate the variable's value was not changed with the new DBG_VALUE (for the same variable), but it was moved around due to the code from [1].

[1] https://github.com/llvm/llvm-project/blob/446082b99f0bd9786d24edd3cd598dc4e5f64371/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp#L590


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:493
+      case VarLoc::EntryValueKind:
+      case VarLoc::EntryValueBackupKind:
+      case VarLoc::EntryValueCopyBackupKind:
----------------
Should we make a different `kEntryValBackupLocation` (and something like `getBackupEntryValuesVarLocs()`)?

The backup locations (`EntryValueBackupKind` and `EntryValueCopyBackupKind`) are invisible/backup/secondary up to the point we lost the primary location of a variable; and then we turn the backup location into entry value location (`EntryValueKind`) which is (further on) used as primary location.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:975
   if (TrySalvageEntryValue) {
-    for (uint64_t ID : OpenRanges.getVarLocs()) {
+    for (uint64_t ID : OpenRanges.getEntryValueVarLocs()) {
       const VarLoc &VL = VarLocIDs[LocIndex::fromRawInteger(ID)];
----------------
Based on my previous comments, I think we need only `getBackupEntryValuesVarLocs()` here.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:1416
   if (isRegOtherThanSPAndFP(*DestRegOp, MI, TRI)) {
-    for (uint64_t ID : OpenRanges.getVarLocs()) {
+    for (uint64_t ID : OpenRanges.getEntryValueVarLocs()) {
       LocIndex Idx = LocIndex::fromRawInteger(ID);
----------------
Here we would need only `getBackupEntryValuesVarLocs()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80684