[PATCH] D30751: [MachineCopyPropagation] Extend pass to do COPY source forwarding

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added inline comments.


================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:188
+                    "Machine Copy Propagation Pre-Register Rewrite Pass", false,
+                    false)
+
----------------
gberry wrote:
> qcolombet wrote:
> > I am wondering if we want to obfuscate that this is really just the same pass with different parameters. I get that the dependencies are also different for the initialization process, but usually we just go with the most constraining one.
> > 
> > Is it worth doing differently here? 
> I took a look at this, and the main problem that comes up when doing it as one pass is the fact that you wouldn't have separate pass IDs that can be used to disable the later run of this pass but not the earlier one (e.g. as is done by NVPTX and WebAssembly targets).  I also think it might make it hard to run the pass in isolation via llc.  If you think these issues are surmountable with the single pass approach, let me know and I'll take another look.
Thanks for double checking. I'm fine with the added complexity given it has an explanation.
Please add comment explaining that in the code.


================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:455
+      if (MOUse.getSubReg())
+        NewUseReg = TRI->getSubReg(NewUseReg, MOUse.getSubReg());
+      // If the original use subreg isn't valid on the new src reg, we can't
----------------
gberry wrote:
> qcolombet wrote:
> > That's invalid per the MachineVerifier
> Can you elaborate on this?  I'm not sure what you are referring to as being invalid.
Sorry. Physical registers don't have subreg indices. The machine verifier checks that.


================
Comment at: lib/CodeGen/MachineCopyPropagation.cpp:349
 
+// Only forward cross-class COPYs into other reversed cross-class COPYs.
+bool MachineCopyPropagation::isForwardableRegClassCopy(MachineInstr &Copy,
----------------
Could you add an example illustrating that?


https://reviews.llvm.org/D30751