[PATCH] D130659: [RISCV] Update lowerFROUND to use masked instructions.

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1919
+      DAG.getConstantFP(MaxVal, DL, ContainerVT.getVectorElementType());
+  SDValue MaxValSplat = DAG.getNode(RISCVISD::VFMV_V_F_VL, DL, ContainerVT,
+                                    DAG.getUNDEF(ContainerVT), MaxValNode, VL);
----------------
craig.topper wrote:
> reames wrote:
> > Why is the explicit splat here needed?  getConstantFP seems to do generate a splat internally, and the old code relied on that behavior.  Why change it to create the scalar constant (explicitly) and then splat (explicitly)?
> The old code used getConstantFP with either a fixed vector type or scalable vector type. For fixed vector it would create a build_vector that would later be converted by lowerBUILD_VECTOR to VFMV_V_F_VL with a VL based on the fixed vector type. For scalable vector getConstantFP would create a SPLAT_VECTOR that would be treated as a VLMAX vfmv.v.f during isel.
> 
> Since we are converting fixed length vectors to scalable in order to use _VL nodes, we need to match how BUILD_VECTOR would be converted. Using getConstantFP with the ContainerVT would create a VLMax splat instead of using the VL from the fixed vector we started with. It would still work since the .vx pattern match in isel ignores the VL on the splat, but if it wasn't folded to a .vx instruction it would create a vsetvli toggle.
Ah yes, I've stumbled into this issue before.

I'm finding myself wondering if we need a version of SPLAT_VECTOR which takes an explicit VL, and represents a splat over those lanes only.  On the other hand, the overlap between BUILD_VECTOR and SPLAT_VECTOR is already confusing enough.  This is probably worthy of some offline discussion; we're not going to magically resolve this here.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130659