[PATCH] D89048: [ARM][LowOverheadLoops] Insert loop start at end of block in more cases

[Sam Tebbs via Phabricator via llvm-commits]

samtebbs added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1083
+    if (FirstNonTerminator != MBB->end()) {
+        if (RDA.hasSameReachingDef(Start, &*FirstNonTerminator, CountReg) &&
+                   RDA.hasSameReachingDef(Start, &*FirstNonTerminator, ARM::LR))
----------------
samparker wrote:
> I seem to remember being concerned that though we've already chosen an InsertPt, we then compare reaching defs against Start instead... In the case where InsertPt != Start, it means there's a mov lr, after Start and so RDA.isReachingDefLiveOut(Start, ARM::LR) shouldn't be true and RDA.hasSameReachingDef(Start, &*FirstNonTerminator, ARM::LR) may also not be true. Would you mind making another patch to compare against InsertPt and then rebase this on top of that?
Good idea. done.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1092
+        auto LastMI = &*MBB->getLastNonDebugInstr();
+        if (RDA.hasSameReachingDef(LastMI, &*InsertPt, CountReg)) {
+            for (auto &Op : InsertPt->operands()) {
----------------
samparker wrote:
> nit: your indentations are too wide.
Cheers


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

https://reviews.llvm.org/D89048