[PATCH] D53137: Scalable vector core instruction support + size queries

[Graham Hunter via Phabricator via llvm-commits]

huntergr added a comment.

Thanks for the reviews; I left a couple of inline comments, and will make the requested changes.



================
Comment at: llvm/include/llvm/CodeGen/ValueTypes.h:214
+      ScalableSize Bits = getScalableSizeInBits();
+      return (Bits.MinSize & 7) == 0;
     }
----------------
rovka wrote:
> Why does this one need to change? We're only looking at MinSize anyway, isn't getSizeInBits good enough?
getSizeInBits will assert (actually, the underlying MVT version for it will) for a scalable vector type, so can't be used.

However, I introduced the 'getMinSizeInBits' function after writing this, so it should be used here.


================
Comment at: llvm/lib/Target/Hexagon/HexagonISelLowering.cpp:1438
   for (MVT VT : MVT::vector_valuetypes()) {
+    // Scalable vectors aren't supported on this backend.
+    if (VT.isScalableVector())
----------------
greened wrote:
> What about other backends?  Do they need similar checks (both here and below)?
I'll take a look, but the Hexagon backend was the only one which asserted when running `make check-all`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D53137