[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/include/llvm/CodeGen/SelectionDAG.h:657-658
 
+  SDValue getStepVector(const SDLoc &DL, EVT ResVT, EVT OpVT,
+                        const APInt Step) {
+    assert(Step.getActiveBits() <= OpVT.getScalarSizeInBits());
----------------
david-arm wrote:
> paulwalker-arm wrote:
> > I guess I've caused this with my previous "don't use a node request" and thus the answer will be no, but I'll ask the question anyway. Can OpVT be dropped here? as it seems inconvenient.
> Yeah, that's right. This arose because of trying to pass in an immediate value here. I'm not sure of a much less ugly way when removing OpVT. If we remove it then I have to create a type every time based upon the current width of Step and assume the caller has set the bit width correctly. I can't just use the element type of ResVT because this takes me back to square one, i.e. that I then need to worry about promoting the type to i32 or i64. I could pass in a ConstantSDNode instead of the {OpVT, Step} pair, which has both the OpVT type and the APInt combined?
Thanks for the information Dave and sorry for messing you around here.  How about we go back to a stock SDValue but add an assert in `getNode` that the second operand is the constant we expect?  This doesn't prevent people using DAG interfaces to transform what we know to be a constant but at least we'll catch is very early if it goes wrong.

I still think this interface is of value as a generic way to create such vectors regardless of vector type.


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

https://reviews.llvm.org/D97299