[PATCH] D92312: [IR] Disallow scalable vectors in ShuffleVectorInst::isExtractSubvectorMask

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes added inline comments.


================
Comment at: llvm/include/llvm/IR/Instructions.h:2281
+    // case.
+    if (isa<ScalableVectorType>(getType()))
+      return false;
----------------
david-arm wrote:
> To be honest, it feels a bit inconsistent that in one variant of the function we assert it's a FixedVectorType, but we permit it in the other. Is it worth making both variants assert we're dealing with a FixedVectorType and changing the callsites to only call for FixedVectorTypes? It looks like we only these functions called from two places.
> To be honest, it feels a bit inconsistent that in one variant of the function we assert it's a FixedVectorType, but we permit it in the other. Is it worth making both variants assert we're dealing with a FixedVectorType and changing the callsites to only call for FixedVectorTypes? It looks like we only these functions called from two places.

I initially implemented these identically to return false for scalable vectors, but then I realised for the other variant it's more of a stretch to incorrectly call it with a scalable vector since it's static and asks for a fixed number of elements. If we don't want the inconsistency I think my preference would be replacing the assert with what we have here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92312