[PATCH] D125335: Pass lamba to MachineCopyProp to determine which MI is treated as Copy instruction.

[Xin Tong via Phabricator via llvm-commits]

adriantong1024 added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:298
+    if (UseCopyInstr || EnableCopyInstr) {
+      PredicateCopy = [](const MachineInstr &MI) {
+        const TargetInstrInfo *TII = MI.getMF()->getSubtarget().getInstrInfo();
----------------
craig.topper wrote:
> craig.topper wrote:
> > Why do you need a lambda inside the pass? Can't you pass around the bool and TII? And have a static function that takes MI, TII, and the bool and does the right thing?
> Or make more of the static functions in the pass, member functions so you don't have to pass around TII and the bool/predicate.
Yes. I thought about it and that has the cost of passing 2 arguments around. while this has the cost of retrieving TII from MI. 


================
Comment at: llvm/test/CodeGen/AArch64/O3-pipeline.ll:202
 ; CHECK-NEXT:       AArch64 load / store optimization pass
+; CHECK-NEXT:       Machine Copy Propagation Pass
 ; CHECK-NEXT:       Workaround A53 erratum 835769 pass
----------------
arsenm wrote:
> Was this not already run? Is this adding a second instance?
Hi @arsenm, Yes this is running the 2nd instance.  We noticed some redundant copy instructions after tail duplication in machine block placement, so we want to run MCP to get rid of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125335