[PATCH] D124655: [IR][CostModel] A scalable vector shuffle can't be an identity shuffle.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/include/llvm/IR/Instructions.h:2151-2153
   /// Return true if this shuffle chooses elements from exactly one source
   /// vector without lane crossings and does not change the number of elements
   /// from its input vectors.
----------------
sdesmalen wrote:
> nit: not an issue with your patch, but reading this comment made me think that a splatvector actually adheres to the criteria listed here:
> * it chooses all elements from exactly one source vector without lane crossings (it always chooses element 0)
> * it doesn't change the number of elements from it's input vectors. (true for `<vscale x K x i32> shufflevector <vscale x K x i32> %ins, <vscale x K x i32> undef, <vscale x K x i32> zeroinitializer`)
> 
> It should actually say "Return true if this shuffle chooses successive elements". Could you maybe update the wording as part of this patch?
I think "without lane crossing" meant the individual elements don't change their position. isSelect() also says "without lane crossing"


================
Comment at: llvm/test/Analysis/CostModel/RISCV/rvv-shuffle.ll:8
 ; CHECK-LABEL: 'vector_broadcast'
 ; CHECK-NEXT:  Cost Model: Invalid cost for instruction: %zero = shufflevector <vscale x 8 x i8> undef, <vscale x 8 x i8> undef, <vscale x 8 x i32> zeroinitializer
 ; CHECK-NEXT:  Cost Model: Invalid cost for instruction: %1 = shufflevector <vscale x 16 x i8> undef, <vscale x 16 x i8> undef, <vscale x 16 x i32> zeroinitializer
----------------
sdesmalen wrote:
> Has the cost-model not yet been implemented for RVV that these costs are all Invalid?
This issue was found while the cost model was being added. It was committed after I uploaded this patch showing a cost of 0. When I rebase this patch it will change from 0 to non-zero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124655