[PATCH] D107210: [RISCV] Support interleaved load lowering

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:971
+  // Ensure the element type is legal.
+  if (ElSize != 8 && ElSize != 16 && ElSize != 32 && ElSize != 64)
+    return false;
----------------
Does this need to check if floating point elements are supported?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:995
+  ScalableVectorType *SVTy;
+  bool isScalableVector = isa<ScalableVectorType>(VTy);
+  if (!isScalableVector) {
----------------
Please fix this clang-tidy warning


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:997
+  if (!isScalableVector) {
+    auto *FVTy = cast<FixedVectorType>(VTy);
+    SVTy =
----------------
This is not the correct way to convert a fixed vector type to a scalable vector type. For large vectors this will create a type that maps to something larger than LMUL=8. We need to map fixed vector types to scalable vector types using the logic from RISCVTargetLowering::getContainerForFixedLengthVector which takes into account a user provided VLEN via -riscv-v-vector-bits-min command line option.


================
Comment at: llvm/test/Transforms/InterleavedAccess/RISCV/interleaved-accesses.ll:8
+; CHECK-NEXT:    [[TMP1:%.*]] = bitcast <16 x i8>* [[PTR:%.*]] to i8*
+; CHECK-NEXT:    [[VLSEGN:%.*]] = call { <vscale x 8 x i8>, <vscale x 8 x i8> } @llvm.riscv.vlseg2.nxv8i8.i64(i8* [[TMP1]], i64 1)
+; CHECK-NEXT:    [[TMP2:%.*]] = extractvalue { <vscale x 8 x i8>, <vscale x 8 x i8> } [[VLSEGN]], 1
----------------
Why is the VL 1? The vector type has 16 elements, the interleave factor is 2. So the VL should be 8.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107210