[PATCH] D97299: [IR][SVE] Add new llvm.experimental.stepvector intrinsic

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:1634-1636
+  SDValue EltCnt = DAG.getVScale(
+      dl, EltVT,
+      APInt(EltVT.getScalarSizeInBits(), LoVT.getVectorMinNumElements()));
----------------
bin.cheng-ali wrote:
> paulwalker-arm wrote:
> > I guess there's no action required for this patch as fixed length vectors are currently excluded, however, if this idiom becomes common then I suggest creating a DAG.getElementCountAsNode(EVT) like function.  That way code like this will just work for both fixed and scalable vectors.
> My question is if it should become common?  I mean if fixed length vector can be handled more efficiently, shall we always lower and handle fixed/scalable length differently, rather than handle them simultaneously in more and more functions, which has no obvious benefit?
I agree and hope we evolve to a more unified set of nodes that work for all vector types.  That said, this is a larger conversation and so today we're trying to add scalable vector support in a way that is a NFC for fixed length vectors.  This patch does add a unified interface in the form of `SelectionDAG::getSepVector(...)` that should allow for portable code regardless of vector type.

SplitVecRes_STEP_VECTOR is specifically linked to `ISD::STEP_VECTOR`, which can only be created for scalable vector types. So whilst this function can be easily extended to cover fixed length vector types, it would not be testable in a way that preserved this "NFC for fixed length vectors" goal.


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

https://reviews.llvm.org/D97299