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

Xiang Zhang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 29 19:32:56 PDT 2023


xiangzhangllvm added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:611
 
+  auto CheckRegClass = [this](Register Def, Register Src) {
+    for (const TargetRegisterClass *RC : TRI->regclasses()) {
----------------
xiangzhangllvm wrote:
> 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.
isForwardableRegClassCopy already try handle cross-class case.
Looks we only need to enhance it:

```
diff --git a/llvm/lib/CodeGen/MachineCopyPropagation.cpp b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
index 218f293a4085..fff2893e2148 100644
--- a/llvm/lib/CodeGen/MachineCopyPropagation.cpp
+++ b/llvm/lib/CodeGen/MachineCopyPropagation.cpp
@@ -539,6 +539,16 @@ bool MachineCopyPropagation::isForwardableRegClassCopy(const MachineInstr &Copy,
   /// so we have reduced the number of cross-class COPYs and potentially
   /// introduced a nop COPY that can be removed.

+  // We don't propagate for SubReg in cross-class COPYs.
+  // For example: (ecx is SubReg of rcx)
+  // renamable $rcx = COPY killed renamable $k0
+  // renamable $k1 = COPY renamable $ecx
+  Register CopyDstReg = CopyOperands->Destination->getReg();
+  Register UseSrcReg = UseICopyOperands->Source->getReg();
+  if (TRI->getRegSizeInBits(CopyDstReg, *MRI) !=
+      TRI->getRegSizeInBits(UseSrcReg, *MRI))
+    return false;
+
   // Allow forwarding if src and dst belong to any common class, so long as they
   // don't belong to any (possibly smaller) common class that requires copies to
   // go via a different class.
```
Let me do more testing, thanks!


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

https://reviews.llvm.org/D147031



More information about the llvm-commits mailing list