[PATCH] D133829: [RISCV] Add cost model for insertelement/extractelement of vector type that should be splitted.

[Jianjian Guan via Phabricator via llvm-commits]

jacquesguan added a comment.

In D133829#3821669 <https://reviews.llvm.org/D133829#3821669>, @reames wrote:

> Can you add some test coverage specifically for the insert/extract case as opposed to relying on updates in lowering of more complicated constructs?

Done, I added in https://reviews.llvm.org/D135534.



================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:795
+          (Index != -1U &&
+           Index <= LT.second.getVectorElementCount().getKnownMinValue()))) {
+      // Otherwise, firstly, we need to spill it to stack.
----------------
reames wrote:
> I believe that for a scalable vector, this effectively returns the M in <vscale x M x iN>.  While correct, this is smaller than the maximum index we can prove in the first vector.  
> 
> (e.g. for +v, M is 1 for i64, but provably valid indexes are 0 and 1)
> 
> Here's an alternative:
> 
> ```
>       unsigned EltSize = VT.getScalarSizeInBits();
>       unsigned MinSize = VT.getSizeInBits().getKnownMinValue();
>       unsigned VectorBitsMin = Subtarget.getRealMinVLen();
>       unsigned MinVLMAX =
>           RISCVTargetLowering::computeVLMAX(VectorBitsMin, EltSize, MinSize);
> ```
> 
Yes, I agree with you, but this cost should obey `DAGTypeLegalizer`'s action which just check with the value of M.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133829