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

Tue Mar 30 07:59:51 PDT 2021

paulwalker-arm added a comment.

In D97299#2658448 <https://reviews.llvm.org/D97299#2658448>, @frasercrmck wrote:

> In D97299#2629320 <https://reviews.llvm.org/D97299#2629320>, @paulwalker-arm wrote:
>
>> I'm still struggling with how best to represent STEPVECTOR's scale operand.  The patch as it currently stands is probably good enough but I do wonder how long we'll get by without needing to implement PromoteIntOp_STEP_VECTOR.  I also wonder if we can take a similar approach as done for INSERT_SUBVECTOR's index operand.
>
> Hey. Sorry to raise a closed thread but this exact issue is making it difficult to support `STEP_VECTOR` on RISC-V. On RV32 we don't have legal i64 but //do// have legal i64 vectors. So we're hitting an assert during `visitStepVector` where we try and create a nxvXi64 STEP_VECTOR with an i32 operand. Are you aware of anything that stops us from changing the requirements to be an integer pointer type? It might limit optimizations but we can always //not// perform DAG combines if it's not safe to do so?

That good feedback, thanks.  The problem is if we ever decide to allow a negative operand.  By forcing the operand to have at least the same bit width as the result's element type we managed to build in some future-proof-ness.  That said I'm not locked into this idea plus we can easily switch the operand to be signed if that make more sense and as you say, not dag combine when it would be unsafe to do so.  So if this is proving problematic for you then I've nothing against making the operand an integer pointer type.

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

https://reviews.llvm.org/D97299