[PATCH] D67539: [ARM][LowOverheadLoops] Add LR def safety check

Mon Sep 16 08:36:16 PDT 2019

dmgreen added a comment.

Will tracking liveness this late be OK? I have a memory of it not being reliable, but that might have been fixed up recently?

And it seems a little odd that the t2DoLoopStart/t2WhileLoopStart wouldn't have any reference to LR in it! But that's what it is. Lets get this problem fixed first.

================
Comment at: lib/Target/ARM/ARMLowOverheadLoops.cpp:122
+  T MI(Begin);
+  do {
+    for (auto &MO : MI->operands()) {
----------------
Can this be something like `for(auto MI : make_range(T(Begin), End))` ?

================
Comment at: lib/Target/ARM/ARMLowOverheadLoops.cpp:155
+  auto IsMoveLR = [](MachineInstr *MI, unsigned Reg) {
+    return MI->getDesc().isMoveReg() &&
+           MI->getOperand(0).isReg() &&
----------------
Is this always just a tMOVr? Would it be better to check that specifically?

================
Comment at: lib/Target/ARM/ARMLowOverheadLoops.cpp:169
+  MachineInstr *PredLRDef =
+    SearchForDef<MachineBasicBlock::reverse_iterator>(Start, MBB->rend(),
+                                                      ARM::LR);
----------------
Is the template type needed, or can it be left implicit?

================
Comment at: lib/Target/ARM/ARMLowOverheadLoops.cpp:182
+  // if they are performing the equilvant mov that the Start instruction will.
+  // To this by scanning forward and backward to see if there's a def of the
+  // register holding the count value. If we find a suitable def, return it as
----------------
To > Do this

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

https://reviews.llvm.org/D67539