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

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 25 13:16:45 PST 2020


arsenm added inline comments.


================
Comment at: llvm/include/llvm/Target/Target.td:1126
+  let isIndirectBranch = 1;
+}
 def BUNDLE : StandardPseudoInstruction {
----------------
void wrote:
> nickdesaulniers wrote:
> > `COPY` also sets `hasNoSchedulingInfo` to `0`. Should `COPY_BR` do that as well?
> Possibly, though I had trouble finding where to place its scheduling info. It seems like `COPY` is special and TableGen adds it by default. I couldn't find the place to do that for `COPY_BR`, but I'll take another look.
Last time I looked at adding a copy variant, the right solution was just isAsCheapAsAMove = 1. Otherwise it would have required adding the COPY_BR to every target's COPY handling


================
Comment at: llvm/lib/CodeGen/SelectionDAG/InstrEmitter.cpp:179
+    const MachineBasicBlock::iterator Term = MBB->getFirstTerminator();
+    if (Term != MBB->end() && Term->getOpcode() == TargetOpcode::INLINEASM_BR)
+      TgtOpc = TargetOpcode::COPY_BR;
----------------
void wrote:
> arsenm wrote:
> > I would expect this to not special case INLINEASM_BR. This should be any value defined by a terminator instruction
> Does this include unconditional jump terminators?
I'm not sure how you could construct a sensible unconditional jump that would require a copy after it. Seems like something to check in the verifier


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