[PATCH] D75098: Add TCOPY, a terminator form of the COPY instr

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:1121
+    return getOpcode() == TargetOpcode::COPY ||
+        getOpcode() == TargetOpcode::COPY_BR;
   }
----------------
void wrote:
> void wrote:
> > arsenm wrote:
> > > void wrote:
> > > > nickdesaulniers wrote:
> > > > > Does this save a few `getOpcode()` calls? The rest of this CL explicitly compares the opcode against `COPY_BR`? I just worry if this might mess up existing callsites of `isCopy` when `getOpcode() == TargetOpcode::COPY_BR;`.
> > > > It's more than just a few. `isCopy()` is used pretty extensively. I want the `COPY_BR` (or `COPY_TERM`) to be exactly like a normal `COPY`, with the only exception that it can come after a terminator.
> > > I think this is potentially hazardous. I'm worried about the peephole optimizer doing things like folding a copy into a tcopy and losing the terminator bit. Can you add some testcases with coalescable tcopy pairs for PeepholeOptimizer and the register coalescer?
> > I'll craft some tests, though note that TCOPY should only be after a terminator, so a COPY should never be merged with it. (I'll add a verifier check to ensure that TCOPY doesn't happen before a terminator.)
> @arsenm I'm struggling a bit to come up with tests that will exercise the peephole optimizer and register coalescer. Do you have any advice on how to do this?
You shouldn't need a special verifier check, it should be caught by the normal terminator before non-terminator check.

I mean something like
%1 = COPY %0
%2 = TCOPY %1

and then maybe mix in some subregistsers? The worry is the COPY will somehow replace the TCOPY



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75098