[PATCH] D158976: [RISCV] Add isCommutable for pseudos without merge operand

[Wang Pengcheng via Phabricator via llvm-commits]

wangpc marked 2 inline comments as done.
wangpc added a comment.

In D158976#4621291 <https://reviews.llvm.org/D158976#4621291>, @michaelmaitland wrote:

> I'm not very familiar with how `isCommutable` works, but this change looks like a good idea as long as `isCommutable` only refers to the `$rs2 and $rs1` operands, and does not say anything about commutability of `$carry, $vl, or $sew`.

Yes. If we don't do anything else but set `isCommutable` to true, the first and second input operand will be commutable. For pseudos whose first operand is merge operand, we need to fix them in `TargetInstrInfo::findCommutedOpIndices` (I'm working on it).



================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:3059
               SchedBinary<"WriteVICALUV", "ReadVICALUV", "ReadVICALUV", mx, forceMasked=1,
                           forceMergeOpRead=true>;
     defm "" : VPseudoBinaryV_XM<m, CarryOut=1, CarryIn=1, Constraint=Constraint>,
----------------
michaelmaitland wrote:
> michaelmaitland wrote:
> > The description says:
> > 
> > > there is no merge operand for their pseudos
> > 
> > but it looks like we are forcing a merge operand.  It looks like `VPseudoBinaryV_VM` defines `VPseudoBinaryCarryIn` which does not have a merge operand. Maybe we should not set forceMergeOpRead here?
> typo: looks like we are forcing a merge operand SchedRead
Yes. I agree with you, it shouldn't be set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158976