[PATCH] D72131: [ARM][LowOverheadLoops] Update liveness info

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Looks interesting. Thanks for splitting it out, it makes it a lot clearer.



================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:42
 
+  class PostOrderTraversal {
+    MachineLoop &ML;
----------------
I believe there is already a PostOrderTraversal, but it deals with whole functions?

Maybe call this PostOrderTraversalForLoop, to distinguish it a little?


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:56
+
+    // Visit all the blocks within the block, as well as exit blocks and any
+    // blocks immediately dominating the header.
----------------
within the block -> within the loop ?


================
Comment at: llvm/test/CodeGen/Thumb2/LowOverheadLoops/disjoint-vcmp.mir:132
   ; CHECK:   frame-setup CFI_INSTRUCTION offset $r4, -16
-  ; CHECK:   $r7 = frame-setup tADDrSPi $sp, 2, 14, $noreg
+  ; CHECK:   dead $r7 = frame-setup tADDrSPi $sp, 2, 14, $noreg
   ; CHECK:   frame-setup CFI_INSTRUCTION def_cfa $r7, 8
----------------
Is this r7 really dead?


================
Comment at: llvm/test/CodeGen/Thumb2/LowOverheadLoops/mov-lr-terminator.mir:111
   ; CHECK:   t2IT 0, 8, implicit-def $itstate
-  ; CHECK:   tPOP_RET 0, killed $cpsr, def $r4, def $pc, implicit killed $itstate
+  ; CHECK:   tPOP_RET 0, killed $cpsr, def dead $r4, def $pc, implicit killed $itstate
   ; CHECK:   renamable $r12 = t2LSRri killed renamable $r3, 1, 14, $noreg, $noreg
----------------
Is r4 really dead here? It's "returned" (kind of) right?


================
Comment at: llvm/test/CodeGen/Thumb2/LowOverheadLoops/while.mir:4
 # TODO: Remove the lr = tMOVr
-# CHECK: body:
-# CHECK: entry:
----------------
It can be good to update these checks in separate NFC patched so the diffs are clearly visible here.


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

https://reviews.llvm.org/D72131