[PATCH] D125856: [WIP] [RISCV] Add cost model for SK_Reverse

[LiqinWeng via Phabricator via llvm-commits]

Miss_Grape added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:191
+    static const CostTblEntry ShuffleTbl[] = {
+        {TTI::SK_Reverse, MVT::nxv1i64, 1},
+        {TTI::SK_Reverse, MVT::nxv2i64, 1},
----------------
craig.topper wrote:
> Why do we need a table to return a 1 for every legal scalable vector type? There's gotta be better ways to do that.
> 
> I don't believe that 1 is the correct answer for RISC-V though. It's a vid.v, a vrsub.vx, and a vrgather.vv if I remember correctly. That's at least a cost of 3 assuming an optimal implementation of vrgather.vv.
> Why do we need a table to return a 1 for every legal scalable vector type? There's gotta be better ways to do that.

CostTableLookup 



================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:272
 
+InstructionCost RISCVTTIImpl::getMemoryOpCost(unsigned Opcode, Type *Ty,
+                                              MaybeAlign Alignment,
----------------
craig.topper wrote:
> Miss_Grape wrote:
> > craig.topper wrote:
> > > This doesn't seem to go with the title of this patch.
> > To ensure that the Load/store's InstructionCost‘s state is set to Valid， when  VF.isScalar() == true
> Can you put it in a different patch that is applied before this one?
I add the test case， Pls review： https://reviews.llvm.org/D125866 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125856