[PATCH] D94981: [LiveDebugVariables] Add cache for SkipPHIsLabelsAndDebug to prevent iterating the same set of PHI/LABEL/Debug instructions repeatedly.

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:1309-1315
+      if (MachineInstr *BI = BBSkipInstsMap[MBB])
+        BeginIt = std::next(MachineBasicBlock::iterator(BI));
+      else
+        BeginIt = MBB->begin();
+      MachineBasicBlock::iterator I = MBB->SkipPHIsLabelsAndDebug(BeginIt);
+      if (I != MBB->begin())
+        BBSkipInstsMap[MBB] = &*std::prev(I);
----------------
dblaikie wrote:
> Could you store iterators in the map instead of pointers? Would save having to do the `&*` dance on the way in and the `MachineBasicBlock::iterator` dance on the way out?
> Also, it'd be good to avoid the extra BBSkipInstsMap lookup, if possible. Would this work?
> ```
> auto& I = BBSkipInstsMap.insert(MBB, MBB->begin()).first->second;
> I = MBB->SkipPHIsLabelsAndDebug(I);
> return I;
> ```
> Could you store iterators in the map instead of pointers? Would save having to do the &* dance on the way in and the MachineBasicBlock::iterator dance on the way out?

Good idea. MachineBasicBlock is an ilist so iterator should be valid after new instruction is inserted if ilist has the same property as std::list. 

> Also, it'd be good to avoid the extra BBSkipInstsMap lookup, if possible. Would this work?
> auto& I = BBSkipInstsMap.insert(MBB, MBB->begin()).first->second;
> I = MBB->SkipPHIsLabelsAndDebug(I);
> return I;

That may not work. If we insert MBB->begin() into the map, after the last call of SkipPHIsLabelsAndDebug, there may be new non-phis/non-labels/non-debuginstrs inserted at the beginning of MBB and the old begin iterator saved in the map will become stale. When we call SkipPHIsLabelsAndDebug next time, it will skip those non-phis/labels/debug instructions which is not welcomed.

Note for the case that the return value of SkipPHIsLabelsAndDebug is not MBB->begin(), we will save std::prev of the returned iterator because the newly inserted instructions after last call of SkipPHIsLabelsAndDebug will be searched next time.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D94981