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

Wed May 11 07:50:42 PDT 2022

adriantong1024 added a comment.

Thanks for the review @dmgreen.

In D125335#3505465 <https://reviews.llvm.org/D125335#3505465>, @dmgreen wrote:

> I thought you said you were just going to write a new pass for this? :)

Feels like MCP can do the job and most likely has a better (than a new peephole) capability to do copy propagation.

>> It's still very unusual to pass a lamba to a pass constructor. Are there other examples? We usually use TII to implement target behavior.
>
>
>
>> fConversion.cpp is one example. There are another example in CodeGen/ I could not find now.
>
> Yeah there are a few other examples of passing a lambda to a pass if you look at the ARMPassConfig. They usually take a Function though I think.
>
> Would it be simpler, given the lambda that is passed from AArch64 at the moment, to just pass a boolean instead to change the behaviour between using TTI->isCopyInstr and MI.isCopy? Similar to the UseCopyInstr option.

Sure. I agree its a bit simpler.

================
Comment at: llvm/test/CodeGen/AArch64/copyprop.mir:106
+---
+# CHECK-LABEL: name: test1_orr_as_copy
+# CHECK: STRBBui $wzr, killed renamable $x8, 36
----------------
dmgreen wrote:
> It might be worth adding an end-to-end .ll file test too, to show that the propagation happen later after MachineBlockPlacement. To show that part is working, and the make sure it doesn't regress again in the future.
Make sense. Will do.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125335