[PATCH] D141747: Resolve a FIXME in MachineCopyPropagation by allowig propagation to subregister uses.

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:598
 
-    // FIXME: Don't handle partial uses of wider COPYs yet.
+    // When the use is a subregister of the COPY, record the subreg index.
+    unsigned SubregIdx = 0;
----------------
What does "subregister of the COPY" mean? Is it subregister of the source or the destination operand?


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:601
+
+    // This can only occur when we are dealing with physical registers.
     if (MOUse.getReg() != CopyDstReg) {
----------------
MCP ever works with physical registers.


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:604
+      SubregIdx = TRI->getSubRegIndex(CopyDstReg, MOUse.getReg());
+      assert(SubregIdx && "Unable to find index of subregister");
     }
----------------
resistor wrote:
> arsenm wrote:
> > This should bail as before rather than assert, the result class might not support the index
> Can you explain how that scenario could arise? I'm unable to picture it, and I didn't hit any cases of it in the test suite.
I suppose it could be in the case of cross-class copy:
```
$a = COPY $b
use $sub_a
```
Here, `$sub_a` is a sub-register of `$a` and its index is `sub_a_idx`.
`$b` may not have `sub_a_idx` subregister index if `$a` and `$b` are in different register classes.



================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:637
 
     MOUse.setReg(CopySrcReg);
+    if (SubregIdx)
----------------
Should be in the `else` branch of the `if` below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141747