[PATCH] D121771: [RISCV] Add basic cost model for vector casting

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:227
+    if (auto SrcVTy = dyn_cast<VectorType>(Src)) {
+      if (!isTypeLegal(Src) || !isTypeLegal(Dst))
+        return BaseT::getCastInstrCost(Opcode, Dst, Src, CCH, CostKind, I);
----------------
Do you intend to use getTypeLegalizationCost to do this properly in the future?


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:244
+      case ISD::ZERO_EXTEND:
+      case ISD::SINT_TO_FP:
+      case ISD::UINT_TO_FP: {
----------------
FP_TO_SINT/UINT is missing


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:246
+      case ISD::UINT_TO_FP: {
+        auto Cost =
+            getCastInstrCost(Opcode, DstVTy->getElementType(),
----------------
Is there any point in getting the scalar cost? Seems like its always 1 and doesn't have a real connection to how the vector ISA works.

I'm also questioning the accuracy of some of the scalar numbers. i8->float requires a pair of shifts to clear the upper bits before doing an iXLen->float conversion for example. So if we were to fix that, it would be wrong for vectors.


================
Comment at: llvm/test/Analysis/CostModel/RISCV/cast.ll:120
+; RISCV64-NEXT:  Cost Model: Found an estimated cost of 1 for instruction:   ret void
+  %v0 = fpext <8 x half>  undef to <8 x float>
+  %v1 = fpext <8 x half>  undef to <8 x double>
----------------
If you're going to test half vectors you should have +zfh on the command line


================
Comment at: llvm/test/Analysis/CostModel/RISCV/cast.ll:256
+; RISCV64-NEXT:  Cost Model: Found an estimated cost of 7 for instruction:   %s0 = sitofp <4 x i8> undef to <4 x half>
+; RISCV64-NEXT:  Cost Model: Found an estimated cost of 1 for instruction:   %s1 = sitofp <4 x i8> undef to <4 x float>
+; RISCV64-NEXT:  Cost Model: Found an estimated cost of 1 for instruction:   %s2 = sitofp <4 x i8> undef to <4 x double>
----------------
How wrong are we intending the costs to be? This i8-float requires 2 vector instructions since the elements sizes are more than a factor of 2 apart.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121771