[PATCH] D150824: [RISCV] Lower experimental_get_vector_length intrinsic to vsetvli for some cases.

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1184
+
+  if (!IsScalable)
+    return true;
----------------
For fixed vectors, we should be able to statically compute this when the ElementWidth * VF is less than VLEN right?  It's fine to do that in a different patch, just want to make sure I'm not missing something.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1187
+
+  if (TripCountVT != MVT::i32 && TripCountVT != Subtarget.getXLenVT())
+    return true;
----------------
Why not i16?  I don't see anything in the implementation which couldn't be handled via an extend?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:6526
+  assert(VF >= 1 && VF <= 64 && isPowerOf2_32(VF) && "Unexpected VF");
+  // We want to use an SEW and LMUL that matches the ElementWidth if possible.
+  unsigned Factor = ElementWidth / 8;
----------------
The "if possible" bit is bugging me here.  Isn't this a hard requirement?  VLMAX should be a function of SEW and LMUL.  Given the result here depends on VLMAX, don't we have a hard requirement on having a recognized SEW/LMUL combination?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:6535
+  unsigned LMulVal = LMulExp >= 3 ? LMulExp - 3 : LMulExp + 5;
+  unsigned SewVal = Log2_32(ElementWidth) - 3;
+
----------------
encodeSEW


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150824