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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

Can you avoid adding a second run by moving the current run? The ad-hoc physreg liveness tracking after allocation is really expensive



================
Comment at: llvm/lib/CodeGen/MachineCopyPropagation.cpp:89
+
+static cl::opt<bool> MCPUseCopyInstr("mcp-use-is-copy-instr", cl::init(false),
+                                     cl::Hidden);
----------------
adriantong1024 wrote:
> lkail wrote:
> > adriantong1024 wrote:
> > > lkail wrote:
> > > > Why do we need this option? Can we extend MCP to check `TII.isCopyInstr` by default?
> > > I've tried to use isCopyInstr by default, but there are cases which it does not work for some architecture. 
> > > 
> > > e.g. ARM
> > > $lr = tMOVr killed $r1, 14, $noreg
> > > tBX_RET 14, $noreg, implicit $r0
> > > 
> > > isCopyInstr sees tMOVr as a COPY instruction, but the use of $lr is not represented in tBX_RET, so the tMOVr is eliminated.
> > > 
> > > So far such problems have not been seen in AArch64.
> > I mean first we can check `MI.isCopy()`, if `MI.isCopy()` returns `false`, we do further check via `TII->isCopyInstr(MI)`. For the initial draft, we can do simple feasibility check(sufficient to cover your case) on `MI` which `TII->isCopyInstr(MI)` returns `true`.
> This is what isCopyInstr does (https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/CodeGen/TargetInstrInfo.h#L1025)
> 
> The problem (my concern) here is some architecture may not be ready to use isCopyInstr to identify what a COPY is. We could slowly enable it for more and more architectures once this option in place.
I am against adding any kind of option here. These are totally undiscoverable and we have too many random flags and hooks out of fear of changing anything on any other architecture


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125335