PATCH: RegisterCoalescer: Fix bug when rematerializing instsructions with subregs

[Quentin Colombet]

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.

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140418/a09160f5/attachment.html>