This patch implements getArithmeticInstrCost for RISCV, supports cost
model for integer and float vector arithmetic instructions.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp | ||
---|---|---|
947 | These costs are incorrect when e.g. a half is used without Zfh. | |
954 | 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. |
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp | ||
---|---|---|
947 | 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. | |
954 | It's a mistake, I changed to SEW related. Or maybe we could just use TCC_Expensive? |
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp | ||
---|---|---|
1053 | You have a bug here - you didn't pass through the last two parameters and thus get the defaults when calling the base class version of the function. I found this when iterating on a local version of this patch with the intention of subsetting and giving feedback on the remaining part. I went ahead and landed the plumbing for this change as d2cf0bd7 |
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp | ||
---|---|---|
1053 | OK, thank you. |
I landed a slightly cleaned up subset of this in db07d79ab0. I left out the div/rem parts because that makes absolutely no sense to me. If you want to move forward with that piece, please move it to it's own patch rebased over tip of tree and explain why that's a reasonable cost model.
p.s. Please close this review. The auto-close didn't work since I had not commandeered the review.
These costs are incorrect when e.g. a half is used without Zfh.