[PATCH] D154245: [RISCV] Remove legacy TA/TU pseudo distinction for binary instructions

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:2543
                                   .add(MI.getOperand(0))
+                                  .addReg(MI.getOperand(0).getReg(), RegState::Undef)
                                   .add(MI.getOperand(1))
----------------
craig.topper wrote:
> reames wrote:
> > craig.topper wrote:
> > > What happens if we don't use undef here?
> > If I remember correctly, you get a verifier error due to a use before definition.
> Shouldn't we be using the register from Operand 1 not Operand 0?
Not unless I'm missing something?

We're forming the non-tied form of the instruction here.  This used to be the unsuffixed (TA) version, and thus the merge was implicitly undef.  With the new representation, we have the policy and merge operands.  We have to explicitly form the undef merge operand.  

Since the operand is undef, it can be any register whose class matches the def, but it conceptually makes the most sense to me to have this be the destination register given that's what it's tied to.  I believe that using 0 or 1 here is NFC, provided it's flagged undef.  If you have a strong preference here, I'm happy to change.



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

https://reviews.llvm.org/D154245