[PATCH] D71007: [ARM][LowOverheadLoops] Remove dead loop update instructions

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/CodeGen/ReachingDefAnalysis.cpp:275
 
+MachineInstr *ReachingDefAnalysis::isRegUsedBefore(MachineInstr *MI,
+    int PhysReg) {
----------------
I would favour just returning a bool here, or renaming the method.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:55
     LowOverheadLoop(MachineLoop *ML) : ML(ML) {
       MF = ML->getHeader()->getParent();
     }
----------------
I think now would be a good time to add an assertion that we're always operating on a loop with a single block.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:671
+  SmallVector<MachineInstr*, 4> Uses;
+  RDA->getReachingLocalUses(LastInstrInBlock, ElemCount, Uses);
+  if (Uses.size()) {
----------------
LastInstrBlock should be Def, right? Looks like you'll need to add another test case to catch this.


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

https://reviews.llvm.org/D71007