[PATCH] D135080: [RISCV] Add patterns for vfmacc for vp.select/vp.merge and vp.fma.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td:1418
 defm : VPatFPMulAddVL_VV_VF<riscv_vfnmsub_vl, "PseudoVFNMSUB">;
+defm : VPatFPMulAccVL_VV_VF<riscv_vfmadd_vl,  "PseudoVFMACC">;
 
----------------
fakepaper56 wrote:
> fakepaper56 wrote:
> > fakepaper56 wrote:
> > > craig.topper wrote:
> > > > What about the VFMSAC, etc.? You can do them in a separate patch if you want but please give a roadmap of other patches you're planning.
> > > > 
> > > > This is also valid for integer vmacc and vnmsac.
> > > Sure, I will add VFMSAC in this patch. And I will add another patch for vmccc and vnmsac.
> > I think `VPatFPMulAccVL_VV_VF` could serve `VFNMSAC` but not `VFMSAC`, since `VFMSAC` uses negative of adden. 
> Maybe I need to use a new opcode `riscv_vfmadd_vl_oneuse` to make sure the fma instruction is only used by `vselect_vl` or `vp_merge_vl`.
> Maybe I need to use a new opcode `riscv_vfmadd_vl_oneuse` to make sure the fma instruction is only used by `vselect_vl` or `vp_merge_vl`.

Yes


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td:1418
 defm : VPatFPMulAddVL_VV_VF<riscv_vfnmsub_vl, "PseudoVFNMSUB">;
+defm : VPatFPMulAccVL_VV_VF<riscv_vfmadd_vl,  "PseudoVFMACC">;
 
----------------
craig.topper wrote:
> fakepaper56 wrote:
> > fakepaper56 wrote:
> > > fakepaper56 wrote:
> > > > craig.topper wrote:
> > > > > What about the VFMSAC, etc.? You can do them in a separate patch if you want but please give a roadmap of other patches you're planning.
> > > > > 
> > > > > This is also valid for integer vmacc and vnmsac.
> > > > Sure, I will add VFMSAC in this patch. And I will add another patch for vmccc and vnmsac.
> > > I think `VPatFPMulAccVL_VV_VF` could serve `VFNMSAC` but not `VFMSAC`, since `VFMSAC` uses negative of adden. 
> > Maybe I need to use a new opcode `riscv_vfmadd_vl_oneuse` to make sure the fma instruction is only used by `vselect_vl` or `vp_merge_vl`.
> > Maybe I need to use a new opcode `riscv_vfmadd_vl_oneuse` to make sure the fma instruction is only used by `vselect_vl` or `vp_merge_vl`.
> 
> Yes
I think it should still be valid for `VFMSAC`. The elements that are masked off or in the tail won't be negated. The other elements will be.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135080