[PATCH] D147031: [MachineCopy] Bug fix sub register machine copy propagation

[Xiang Zhang via Phabricator via llvm-commits]

xiangzhangllvm added a comment.

In D147031#4229099 <https://reviews.llvm.org/D147031#4229099>, @barannikov88 wrote:

> It crashes on the test from D146930 <https://reviews.llvm.org/D146930>.

Sorry for late respond. Let me take a look on your test. 
Anyway this bug affect our critical tests，I +1 for let D146930 <https://reviews.llvm.org/D146930> first in.
I also comment the risk in D146930 <https://reviews.llvm.org/D146930>.



================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:611
 
+  auto CheckRegClass = [this](Register Def, Register Src) {
+    for (const TargetRegisterClass *RC : TRI->regclasses()) {
----------------
lkail wrote:
> This looks similar to what `isForwardableRegClassCopy` does? Can we extend `isForwardableRegClassCopy` to contain subregindex?
Sound good, we can check it in that function.


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:662
+            TRI->getRegSizeInBits(CopySrcReg, *MRI) &&
+        CheckRegClass(CopyDstReg, CopySrcReg)) {
       SubregIdx = TRI->getSubRegIndex(CopyDstReg, MOUse.getReg());
----------------
barannikov88 wrote:
> All registers in a register class have the same sizes. The check for size above is redundant.
> 
Make sense!


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

https://reviews.llvm.org/D147031