[PATCH] D133007: [RISCV] Add cost model for vector insert/extract element.

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

Adding the insertelement handling caused a lot more test diffs.  For ease of review, would you mind restricting this patch to extractelement and then doing insertelement in a follow up patch?



================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:744
+      Opcode != Instruction::InsertElement)
+    BaseT::getVectorInstrCost(Opcode, Val, Index);
+
----------------
I think you're missing a return here.




================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:788
+    // For insertelement, we need the following instructions:
+    // vsetvli a2, zero, e8, m1, ta, mu (not count)
+    // vmv.s.x v8, a0
----------------
For a follow up patch...

I think we can do better here using logical ops on the mask vector.  Haven't fully worked through the sequence, but generating a mask with one bit set is fairly cheap, and then we're just doing bit toggles. 


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:825
+  }
+  return BaseCost + SlideCost;
+}
----------------
I think we're missing something here.  Specifically, what is the appropriate cost for an illegally typed vector which has been split?  Don't we need to account for the cost of figuring out which sub-vector to extract from/insert into?  For constants, this should fold away, but for a non-constant index doesn't this require some logic?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133007