The patch adds very basic cost model for masked memory op on scalable vector.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp | ||
---|---|---|
173 | That seems like not included the memory access cost? how about return getMemoryOpCost (Opcode, Src, Alignment, AddressSpace, CostKind) here? |
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp | ||
---|---|---|
173 | Thanks, I fixed it |
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp | ||
---|---|---|
169 | How come we're not adding support for fixed-length vectors at the same time? The commit title/description should make it clear that this is scalable-only. | |
llvm/test/Analysis/CostModel/RISCV/masked_ldst.ll | ||
27 | What do you mean by "legal" here? The test isn't testing a configuration in which fixed-length vectors are supported by the backend. | |
47 | Again, what's illegal about these types? | |
92 | This comment says that these types are legal, but the expected output is that there's an "Invalid cost"? That doesn't sound quite right. | |
100 | Again, what's illegal about these? |
LGTM but I would like to let @frasercrmck did final approve.
llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp | ||
---|---|---|
173 | That's pretty HW implementation dependent, so I guess this implementation is good enough for now, and let other RISC-V vendor to fill up their cost in future. |
LGTM. As @kito-cheng said, this is a good starting point in case vendors want to start customizing things. Cheers. Sorry for the slow response.
This seems to fail on a couple of bots (e.g. https://lab.llvm.org/buildbot/#/builders/117/builds/5197), but you may not have gotten emails for those due to an unrelated breakage.
How come we're not adding support for fixed-length vectors at the same time? The commit title/description should make it clear that this is scalable-only.