[PATCH] D16434: Fix ARM load/store opt live reg computing

[Matthias Braun via llvm-commits]

MatzeB added a comment.

In http://reviews.llvm.org/D16434#333985, @weimingz wrote:

> In http://reviews.llvm.org/D16434#333966, @MatzeB wrote:
>
> > In http://reviews.llvm.org/D16434#333962, @weimingz wrote:
> >
> > > At first, I thought LR was just a corner case that ld/st opt didn't handled correctly.
> > >  Then, I find another case where R4 is used in exiting BB and not restored.
> > >
> > > Before ld/st opt pass:
> > >
> > > BB#3: derived from LLVM BB %if.end
> > >
> > >       Live Ins: %R0 %R1 %R2 %R3 %R12 %LR %R7 %R5 %R4
> > >       Predecessors according to CFG: BB#2 BB#1
> > >   	t2STRi12 %R2<kill>, %R1, 20, pred:14, pred:%noreg; mem:ST4[%21]
> > >   	t2STRi12 %R3<kill>, %R1, 24, pred:14, pred:%noreg; mem:ST4[%22]
> > >   	t2STRi12 %R12<kill>, %R1, 28, pred:14, pred:%noreg; mem:ST4[%23]
> > >   	t2STRi12 %R0<kill>, %R1<kill>, 32, pred:14, pred:%noreg; mem:ST4[%24]
> > >   	tBX_RET pred:14, pred:%noreg
> > >  
> > >
> > >
> > > After ld/st opt pass:
> > >  BB#3: derived from LLVM BB %if.end
> > >
> > >       Live Ins: %R0 %R1 %R2 %R3 %R12 %LR %R7 %R5 %R4
> > >       Predecessors according to CFG: BB#2 BB#1
> > >   	%R4<def> = t2ADDri %R1, 20, pred:14, pred:%noreg, opt:%noreg
> > >   	t2STMIA %R4<kill>, pred:14, pred:%noreg, %R2<kill>, %R3<kill>, %R12<kill>   ==> R4 is clobbed
> > >   	t2STRi12 %R0<kill>, %R1<kill>, 32, pred:14, pred:%noreg; mem:ST4[%24]
> > >   	tBX_RET pred:14, pred:%noreg
> >
> >
> > But LivePhysRegs::addLiveOuts should add all the callee saves! Does maybe MBB->isReturnBlock() return false somehow in your case?
>
>
> Oh. currently in addLiveOuts:
>
>   if (!MBB->isReturnBlock()) {
>     // The return block has no successors whose live-ins we could merge
>     // below. So instead we add the callee saved registers manually.
>     for (const MCPhysReg *I = TRI->getCalleeSavedRegs(&MF); *I; ++I)
>       addReg(*I);
>   
>   
>
> Looks we should flip the condition to
>  if (!MBB->isReturnBlock())


Oh, right. I'm surprised nobody has hit a bug because of this before.


Repository:
  rL LLVM

http://reviews.llvm.org/D16434