[PATCH] D67794: [MachineCopyPropagation] Extend MCP to do trivial copy backward propagation

[Kai Luo via Phabricator via llvm-commits]

lkail marked an inline comment as done.
lkail added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:772
+    if (MODef.getReg() != Src)
+      continue;
+
----------------
qcolombet wrote:
> Should this be an assert?
> 
> Put differently, when can we find a backward copy for MODef.getReg() when the source is not MODef.getReg()?
Yes, you are right. I supposed following code might not be handled correctly(to illustrate, might not be perfect)
```
name: foo
alignment:       2
tracksRegLiveness: true
frameInfo:
  maxAlignment:    16
  maxCallFrameSize: 0
stack:
  - { id: 0, type: spill-slot, size: 16, alignment: 16 }
body: |
  bb.0.entry:
    liveins: $r0, $r1
    renamable $q0 = MVE_VLDRWU32 renamable $r0, 80, 0, $noreg :: (load 16 from %stack.0, align 4)
    renamable $s4 = COPY renamable killed $s0
    MVE_VSTRWU32 killed renamable $q1, killed renamable $r1, 0, 0, $noreg :: (store 16 into %stack.0, align 128)
    tBX_RET 14, $noreg
```
I thought `$s0` might be propagated(thought it won't pass `isBackwardPropagatableRegClassCopy` later). However, in `findAvailBackwardCopy` we have already checked `!TRI.isSubRegisterEq(AvailCopy->getOperand(1).getReg(), Reg))`, thus `$s0` won't be propagated.


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

https://reviews.llvm.org/D67794