[PATCH] D133552: [RISCV] Add cost model for integer and float vector arithmetic instructions.

[Jianjian Guan via Phabricator via llvm-commits]

jacquesguan added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:791
+  case ISD::FNEG:
+    return LT.first * 1;
+
----------------
reames wrote:
> These costs are incorrect when e.g. a half is used without Zfh.
There is a check above the switch:
```
  if (!LT.second.isVector())
    return BaseT::getArithmeticInstrCost(Opcode, Ty, CostKind, Op1Info, Op2Info,
                                         Args, CxtI);
```
I think this would early exit if we do not support the element type of the vector.


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:798
+  case ISD::FDIV: {
+    unsigned VL = getEstimatedVLFor(cast<VectorType>(Ty));
+    return LT.first * VL;
----------------
reames wrote:
> This doesn't match the cost of any hardware I'm aware of.  You need to justify why this is correct and probably we need some cpu flag if you know of hardware which behaves like this.  
It's a mistake, I changed to `SEW` related. Or maybe we could just use `TCC_Expensive`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133552