[PATCH] D117079: [RISCV] Improve i64 splat vector lowering in RV32.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2219
+      MVT InterVT =
+          MVT::getScalableVectorVT(MVT::i32, 2 * VT.getVectorNumElements());
+      // TODO: if vl <= min(VLMAX), we can also do this. But we could not
----------------
frasercrmck wrote:
> I may be missing something, but isn't `VT.getVectorNumElements` problematic with scalable vector types? I'm surprised this doesn't warn/crash.
> 
> Also I'm not sure whether this method should assume that `VT` is scalable. Should it be able to produce a fixed-vector type if passed one?
> Also I'm not sure whether this method should assume that `VT` is scalable. Should it be able to produce a fixed-vector type if passed one?

Assuming scalable is ok. Other parts of this code are creating _VL nodes which must use a scalable type.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2219
+      MVT InterVT =
+          MVT::getScalableVectorVT(MVT::i32, 2 * VT.getVectorNumElements());
+      // TODO: if vl <= min(VLMAX), we can also do this. But we could not
----------------
craig.topper wrote:
> frasercrmck wrote:
> > I may be missing something, but isn't `VT.getVectorNumElements` problematic with scalable vector types? I'm surprised this doesn't warn/crash.
> > 
> > Also I'm not sure whether this method should assume that `VT` is scalable. Should it be able to produce a fixed-vector type if passed one?
> > Also I'm not sure whether this method should assume that `VT` is scalable. Should it be able to produce a fixed-vector type if passed one?
> 
> Assuming scalable is ok. Other parts of this code are creating _VL nodes which must use a scalable type.
> I may be missing something, but isn't `VT.getVectorNumElements` problematic with scalable vector types? I'm surprised this doesn't warn/crash.

It happens to work because of a FIXME in getVectorNumElements(). It just returns getVectorMinNumElements().

This code should use `MVT::getVectorVT(MVT::i32, 2 * VT.getVectorElementCount())` I think



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117079