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

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet accepted this revision.
qcolombet added inline comments.
This revision is now accepted and ready to land.


================
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.
+
----------------
arsenm wrote:
> qcolombet wrote:
> > arsenm wrote:
> > > 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?
> > > pull what?
> > 
> > Pull off :P.
> > 
> > I mean it may not be worth spending a lot of effort to support non pure copy everywhere.
> > I.e., Heroically try to implement the full support everywhere for a negligible gain.
> I kind of think we should just get rid of SUBREG_TO_REG entirely. If you wanted to depend on those bits, you should have had to define them in the first place. However doing that is going to be a lot of work. I still haven't found anywhere making use of the "assert 0" property, if you really needed that it would look like AssertZext (which I would hope would be all taken care of before selection is complete)
+1
Also AFAICT, the lowering in ExpandPostRAPseudo is also broken with respect to preserving the bits when the source and target registers are not the same (super) register.


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

https://reviews.llvm.org/D156346