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

[Sam Tebbs via Phabricator via llvm-commits]

samtebbs marked 3 inline comments as done.
samtebbs added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:1086
         InsertPt = FirstNonTerminator;
+      else {
+        // Try putting the loop start at the end of the block even if the count or lr are live-out.
----------------
samparker wrote:
> samparker wrote:
> > Can this logic be simplified? I don't think we should need to be concerned with LR, or at least the top-level conditional block should be guard with:
> > 
> > ```
> > if (FirstNonTerminator == MBB->end() && RDA.isReachingDefLiveOut(Start, ARM::LR)
> > ```
> > Then it looks like what we just need to check whether InsertPt kills CountReg?
> ... along with your existing check for RDA.hasSameReachingDef(LastMI, &*InsertPt, CountReg).
Agreed that it can be simplified. Thanks for pointing that out!


================
Comment at: llvm/test/CodeGen/Thumb2/LowOverheadLoops/it-block-chain-store.mir:146-147
   ; CHECK:   t2STRi12 renamable $lr, killed renamable $r3, 0, 14 /* CC::al */, $noreg :: (store 4 into %ir.iter.addr)
-  ; CHECK:   $lr = t2DLS killed renamable $lr
   ; CHECK:   $r2 = tMOVr killed $lr, 14 /* CC::al */, $noreg
+  ; CHECK:   dead $lr = MVE_DLSTP_32 killed renamable $r12
   ; CHECK: bb.1.do.body:
----------------
dmgreen wrote:
> These tests are hard to be sure, but it looks like the old version was doing something with lr but that might now be a different value?
Indeed, I missing an important check. Thanks.


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

https://reviews.llvm.org/D89048