[PATCH] D95659: [RISCV] Initial support of LoopVectorizer for RISC-V Vector.

[Vineet Kumar via Phabricator via llvm-commits]

vkmr added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:20
+  "riscv-vector-bits-max",
+  cl::desc("The assumed maximum vector bits."),
+  cl::init(0), cl::Hidden);
----------------
Minor nit: Reword the description for more clarity - may be something like "Maximum vector register size in bits"?


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:105
+  if (ST->hasStdExtV())
+    return MaxVectorLen / RISCVVType::RVVBitsPerBlock;
+  return BaseT::getMaxVScale();
----------------
Nit: Use call to `getRegisterBitWidth()` here instead of `RISCVVType::RVVBitsPerBlock`. (Or implement and use `getMinVectorRegisterBitWidth()`)


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h:52-55
+  unsigned getRegisterBitWidth(bool Vector) const {
+    if (Vector) {
+      if (ST->hasStdExtV())
+        return RISCVVType::RVVBitsPerBlock;
----------------
If I understand correctly, the assumption behind this code is that a **single** vector register is of size `vscale x RVVBitsPerBlock` and ignore the idea (for now?) of having register groups, i.e LMUL>1. 
Unless we are ignoring register grouping for now, from Loop Vectorizer's perspective it would make sense to view the register group size as the real register size, specially for computing a feasible VF based on register usage.
Since the documentation of `getRegisterBitWidth()` defines it to be "The width of the largest scalar or vector register type", it might be more accurate to use `getMinVectorRegisterBitWidth()` to return `RISCVVType::RVVBitsPerBlock` and `getRegisterBitWidth()` to return  `getMinVectorRegisterBitWidth() * MAX_LMUL`. (I am not considering fractional LMUL here.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95659