[PATCH] D156346: CodeGen: Disable isCopyInstrImpl if there are implicit operands

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetInstrInfo.h:1044
     if (MI.isCopy()) {
+      // TODO: Should validate implicit operands here?
       return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
----------------
qcolombet wrote:
> Maybe that should be part of the verifier?
Hypothetically you can still end up with the same implicit-def case as the mov, if you were to coalesce a copy into SUBREG_TO_REG. I believe all the instances which use SUBREG_TO_REG are fed by non-copy sources, so not sure if this happens in practice. The verifier probably should verify there are no implicit-defs at least




================
Comment at: llvm/include/llvm/CodeGen/TargetInstrInfo.h:1053
+    // instruction with an undef subregister def, and a full register
+    // implicit-def appended to the operand list.
+
----------------
qcolombet wrote:
> Like I said in https://reviews.llvm.org/D156345, there was already a distinction between pure copies (`isCopy`) and copies with additional stuff that may require careful handling (`isCopyLike`).
> 
> Following this distinction, I feel that either:
> - this is the right fix, or
> - we need to make all the places that deals with isCopyInstr robust w.r.t. `isCopyLike` semantic, which may not be worth it to pull
> worth it to pull

pull what?


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

https://reviews.llvm.org/D156346