[PATCH] D94242: [RISCV] Add scalable vector fcmp ISel patterns

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck marked 2 inline comments as done and an inline comment as not done.
frasercrmck added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td:410
+foreach fvti = AllFloatVectors in
+  def : Pat<(fvti.Vector (splat_vector fvti.ScalarRegClass:$rs1)),
+            (!cast<Instruction>("PseudoVFMV_V_F_"#fvti.LMul.MX)
----------------
craig.topper wrote:
> Should we use PseudoVMV_V_I_ for splat_vector fpimm0?
Yeah, good idea, though the `fmv` intrinsic splat uses `PseudoVMV_V_X` with `X0` and I think it makes sense to make them do the same thing. Do you have a preference? The `V_I` feels better to me, intuitively.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td:361
+
+defm "" : VPatFPSetCCSDNode_VV<SETGT,  "PseudoVMFLT", /*swap*/1>;
+defm "" : VPatFPSetCCSDNode_VV<SETOGT, "PseudoVMFLT", /*swap*/1>;
----------------
craig.topper wrote:
> frasercrmck wrote:
> > craig.topper wrote:
> > > What if we expanded SETGT/SETOGT/SETGE/SETOGE and pattern matched VF from the LHS of SETLT/SETOLT/SETLE/SETOLE. Would that be a net reduction in patterns since we'll lose some of the VV patterns?
> > Possibly. Though I think I prefer it this way as it's more explicit than relying on the expansion to do what we want. Are you thinking more about wins in terms of the size of the generated tables?
> Yeah I was trying to minimize the table size. I thought it would also make VF match easier since we would only need one pattern for each instruction except EQ/OEQ/NE/UNE. I think we already use the Expand for scalar nodes if I recall correctly.
Good point. I have done that now: take a look. I think the table was slightly smaller (not sure how you typically measure that) in the interim, but it's a net-increase with the support for both VF and FV.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94242