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

[Bill Wendling via Phabricator via llvm-commits]

void marked an inline comment as done.
void added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineInstr.h:1121
+    return getOpcode() == TargetOpcode::COPY ||
+        getOpcode() == TargetOpcode::COPY_BR;
   }
----------------
arsenm wrote:
> 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
> 
This isn't an issue as far as I can see (at least the kind of example you're referring to). A TCOPY can become a COPY if appropriate. The only time it can't become a COPY is if it occurs after a terminator, in which case it should be caught by current verifier checks. I *don't* think a COPY should become a TCOPY though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75098