[PATCH] D55872: RegBankSelect: Fix copy insertion point for terminators

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 20 15:48:39 PST 2018


arsenm added a comment.

In D55872#1337787 <https://reviews.llvm.org/D55872#1337787>, @qcolombet wrote:

> Hi Matt,
>
> > I find the existing code here suspect for the case where it's looking for terminators that modify the register. It's going to insert a copy in the middle of the terminators, which isn't allowed (it might be necessary to have a COPY_terminator if anybody actually needs this).
>
> This patch does not address that part right?


No, I switched it to an assert in version 2

> And I agree, this is wrong, we would need to do some basic block splitting.
> 
> Looking back at this code, I wonder if instead of being "clever" about the placement of split point we shouldn't always put then right after the definition of the related instruction and let the machine sinking do the right placement.

I thought about this, but figured since in this case it's most likely for not really copyable condition registers it might end up being problematic


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

https://reviews.llvm.org/D55872





More information about the llvm-commits mailing list