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

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added a comment.

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?
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.

Cheers,
-Quentin



================
Comment at: lib/CodeGen/GlobalISel/RegBankSelect.cpp:736
+
+      addInsertPoint(*It, /*Before*/ false);
       return;
----------------
Add a comment that at this point we are sure to be right before the first terminator.


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

https://reviews.llvm.org/D55872