[PATCH] D69953: MCP: Fixed bug with dest overlapping copy source

[Tim Renouf via Phabricator via llvm-commits]

tpr marked 3 inline comments as done.
tpr added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:439-442
+    for (const MachineOperand &MO : MI.defs()) {
+      if ((Overlap = TRI->regsOverlap(MO.getReg(), CopySrcReg)))
+        break;
+    }
----------------
arsenm wrote:
> This will miss implicit defs. Should be checking MI.modifiesRegister maybe?
Good idea. Makes the code simpler too.


================
Comment at: llvm/test/CodeGen/AMDGPU/mcp-overlap-after-propagation.mir:20
+    renamable $sgpr8 = COPY killed renamable $sgpr2
+    renamable $sgpr60_sgpr61_sgpr62_sgpr63_sgpr64_sgpr65_sgpr66_sgpr67 = S_LOAD_DWORDX8_IMM renamable $sgpr8_sgpr9, 144, 0, 0 :: (invariant load 32, align 16, addrspace 4)
+    renamable $sgpr88_sgpr89_sgpr90_sgpr91_sgpr92_sgpr93_sgpr94_sgpr95 = COPY killed renamable $sgpr60_sgpr61_sgpr62_sgpr63_sgpr64_sgpr65_sgpr66_sgpr67
----------------
arsenm wrote:
> Should add another case where the relevant def is an implicit def
I don't think I can test that. The bug only actually has an effect if the instruction that the copy is being folded into is itself a copy, and I can't think of a way then of there being an implicit def that overlaps the source of the first copy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69953