[PATCH] D107957: [LegalizeTypes][VP] Add splitting support for binary VP ops

Fri Aug 20 08:16:16 PDT 2021

frasercrmck marked 2 inline comments as done.
frasercrmck added inline comments.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1190
+  EVT EVLVT = EVL.getValueType();
+  unsigned HalfMinNumElts = VecVT.getVectorMinNumElements() / 2;
+  SDValue HalfNumElts =
----------------
RKSimon wrote:
> frasercrmck wrote:
> > RKSimon wrote:
> > > Should we need to assert this isKnownEven like we do in EVT::getHalfNumVectorElementsVT ?
> > I was unsure. For some reason I was under the impression that `SplitVecRes_BinOp` and splitting in general could only be called on even-sized vectors. And because `VecVT` is the result type of the split node in question, we know it's even. Is that not the case?
> I think an assert will do no harm and will show we've at least thought about it.
Yeah good idea. Cheers.

================
Comment at: llvm/test/CodeGen/RISCV/rvv/vadd-vp.ll:1627
+; FIXME: The first vadd.vi should be able to infer that its AVL is equivalent to VLMAX.
+; FIXME: The second vsetvli is incorrectly preserving the VL on RV64, rather
+; than setting it to 0 as in RV32.
----------------
@craig.topper, do you have any thoughts about this issue? I don't think it's specific to this patch but rather is just exposed by it. We could probably generated a reduced test case on the current `main`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107957