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

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
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
----------------
dmgreen wrote:
> samparker wrote:
> > samparker wrote:
> > > dmgreen wrote:
> > > > Is this r7 really dead?
> > > Yeah, this is a weird one, it definitely doesn't look dead. I'll look more into CFI_INSTRUCTION.
> > So this looks okay to me, just because the input code labels tPUSH as killing its operands even though the CFI pseudo instructions use them. LiveRegs works on MachineInstrs, whereas CFI is some pseudo MC instruction which doesn't even contain MC operands.
> Perhaps put another way, what does an instruction with a dead def and no side-effects mean? Does it mean we can remove it? What about the frame-setup flag? Does that mean we can't remove it?
> 
> The recomputeLivenessFlags seems to already be used in ifcvt. Does this already happen in the same way there? Looking at tADDrSPi, it might have hasSideEffects set, because it otherwise causes miscompiles. Which might suggest that this isn't correct, but that it's a known issue at least.
I went looking in IfCvt to see if I could get this to produce the same thing (it's the other place that uses this function, and does seem to run it on entry blocks after frame lowering). I couldn't see any examples of it marking registers as dead though.

Then I tried regenerating this function (from the IR above) and it was quite different. I think mostly because the -frame-pointer option has changed. With -frame-pointer=all the code is more similar, but the PUSH above is saving r7 (which kind of makes sense, it's pushed two lines up).

Any chance this file might have been modified to the point that it is not really valid any more?


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

https://reviews.llvm.org/D72131