[PATCH] D80384: [SVE] Add flag to specify SVE register size, using this to calculate legal vector types.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:3500
+  if (VT.getVectorElementType() == MVT::i1)
+    return false;
+
----------------
Restricting i1 types like this seems a little weird to me.  We don't need to pretend to be NEON. We want these types to be legal for similar reasons we want the other fixed vector types to be legal.  I guess we could leave the check for now if it makes it easier to get simple testcases working, but I don't think this is what we want long-term.

For non-i1 types, should we check the element type is legal?  integer_fixedlen_vector_valuetypes() and fp_fixedlen_vector_valuetypes() can return some exotic types.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:3512
+  // the base fixed length SVE support in place.
+  if (!VT.isPow2VectorType())
+    return false;
----------------
Non-power-of-two types are an extra complication, yes; better to leave it out for now.


================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:345
+
+unsigned AArch64Subtarget::getMaxSVEVectorSizeInBits() const {
+  assert(HasSVE && "Tried to get SVE vector length without SVE support!");
----------------
We probably want function attributes?

It looks like this patch doesn't use the max size; what do you anticipate using it for?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80384