[PATCH] D125335: Give option to use isCopyInstr to determine which MI is treated as Copy instruction in MCP

Tue May 17 10:14:19 PDT 2022

adriantong1024 added a comment.

In D125335#3519829 <https://reviews.llvm.org/D125335#3519829>, @lkail wrote:

>> The problem (my concern) here is some architecture may not be ready to use isCopyInstr to identify what a COPY is.
>
> IIUC, `isCopyInstrImpl` hasn't been override by many targets and returns `None`, I think it doesn't affect these targets in MCP. Even when `MI.isCopy()` returns false, and then `isCopyInstr` returns true, you still have chance to decide if it's feasible to perform propagation based on this MI's operands' flags and etc. As I have said, you can implement conservative checks in your first patch(sufficient to cover your cases) and enhance it in following patches.

Hi Kai

I agree that isCopyInstrImpl has not been overriden by many targets. However, in case of ARM, I have observed a problem calling MCP with isCopyInstr(). While this could be fixed by checking additional things in MCP, but this would make MCP more complex and target specific.

Another way is to implement specific AArch64 optimizations to handle my case, but I feel MCP is pretty well written and would be great if we can re-use its code.

Right now MCP+MI.isCopy() runs for all architecture, but MCP+isCopyInstr only runs for AArch64. This could be extended to run on other architectures once tested on the architectures.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125335