[PATCH] D143754: [MachineInstr] Introduce generic predicated copy opcode

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/include/llvm/Support/TargetOpcodes.def:114
+/// Some targets require it for special handling certain register copies.
+  HANDLE_TARGET_OPCODE(PRED_COPY)
+
----------------
So what is the result when the predicate is false?.

(That kind of question does not exist for any existing generic opcodes afaik, because this would be the first predicable generic opcode.)


================
Comment at: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp:204
   case TargetOpcode::COPY:
+  case TargetOpcode::PRED_COPY:
   case TargetOpcode::G_PHI:
----------------
Here we have a typical example where I think it gets complicated to say that PRED_COPY is predicable. If the predicate is false, then you can't guarantee anything about the known bits. OTOH, if the predicate is guaranteed to be true, then there is no need to have a PRED_COPY.


================
Comment at: llvm/lib/CodeGen/TwoAddressInstructionPass.cpp:1927
     // Insert the sub-register copy.
     MachineInstr *CopyMI = BuildMI(*MI.getParent(), MI, MI.getDebugLoc(),
+                                   TII->get(TII->getCopyOpcode()))
----------------
Here is a general comment to about all these places that now may or may not create a predicated instruction.

In our downstream target all predicated instructions also have extra operands. So whenever we create a predicated instruction those predicate operands should be added to the instruction. But that won't happen for any of these places where you use getCopyOpcode(). That is of course not a problem as long as we avoid using PRED_COPY for our target. I'm just saying that this probably wouldn't work for any target, depending on how isPredicable=1 is handled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143754