PATCH: RegisterCoalescer: Fix bug when rematerializing instsructions with subregs

[Tom Stellard]

On Fri, Apr 18, 2014 at 03:43:55PM -0700, Quentin Colombet wrote:
> Hi Tom,
> 
> I’ve looked at your patch and it looks like a workaround to a more general issue.
> 
> I’ve applied your second patch and looked at what happen with the rematerialization in that case.
> 
> Unless I am not looking at the right fragment, the copy you are trying to avoid to rematerialize is:
>   %vreg169<def> = COPY %vreg91:sub0_sub1; SReg_64:%vreg169 SReg_128:%vreg91
> 
> And the related rematerializable definition is:
>   %vreg91:sub0_sub1<def,read-undef> = S_MOV_B64 0; SReg_128:%vreg91
> 
> Thus, one would expect to rematerialize this into:
>   %vreg169:sub0_sub1<def,read-undef> = S_MOV_B64 0; SReg_128:%vreg169
> 
> Which the current code should correctly handle.
> 
> However, in your case, the instruction that have been picked up for rematerialization is:
>   %vreg91:sub3<def> = S_MOV_B32 61440; SReg_128:%vreg91
> 
> Which is the same vreg91, but not the right subregister. So looks like there is a liveness or liveness-update bug with the subregisters.
> 

Are you sure LLVM ToT does sub-register liveness tracking?  I thought liveness was
tracked by super-register only.

-Tom

> Could you please track this down to make sure we understand the problem?
> 
> Now, in the mean time, we may want to provide a workaround (and fix a PR to remember we added a workaround).
> Your workaround is appropriate with the following fix:
> +  // The COPY src reg and the DefMI dst reg must have the same subreg index
> +  if (CopyMI->getOperand(1).getSubReg() != DefMI->getOperand(0).getSubReg())
> +    return false;
> 
> I.e., instead of checking the sub register of CopyDstOperand, you have to check the sub register of copy *src* operand (like you said in your comment), i.e., 1 instead of 0.
> 
> Thanks,
> -Quentin
> 
> On Apr 16, 2014, at 3:13 PM, Tom Stellard <tom at stellard.net> wrote:
> 
> > Hi,
> > 
> > This patch fixes a bug in the register coalescer where it was incorrectly
> > rematerializing instructions that define sub-regs.
> > 
> > I've also attached the R600 patch which uncovers this bug.
> > 
> > Please Review.
> > 
> > -Tom
> > <0001-RegisterCoalescer-Fix-bug-when-rematerializing-insts.patch><0002-R600-SI-Improve-chances-of-rematerializing-during-re.patch>_______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>