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

Bill Wendling via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 10 17:00:21 PDT 2020


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;
   }
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75098





More information about the llvm-commits mailing list