Lower interleaved load to segment load intrinsic.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Don't you need to add the InterleavedAccess pass to RISCVTargetMachine?
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
1231 | These intrinsics only have SelectionDAG support for scalable vector types. And they take VL as an operand. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
1289 | Please address these clang-format warnings |
llvm/test/Transforms/InterleavedAccess/RISCV/interleaved-accesses.ll | ||
---|---|---|
6 | Why are we only testing one factor? |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
1201 | Does this need to check if floating point elements are supported? | |
1225 | Please fix this clang-tidy warning | |
1227 | This is not the correct way to convert a fixed vector type to a scalable vector type. For large vectors this will create a type that maps to something larger than LMUL=8. We need to map fixed vector types to scalable vector types using the logic from RISCVTargetLowering::getContainerForFixedLengthVector which takes into account a user provided VLEN via -riscv-v-vector-bits-min command line option. | |
llvm/test/Transforms/InterleavedAccess/RISCV/interleaved-accesses.ll | ||
9 | Why is the VL 1? The vector type has 16 elements, the interleave factor is 2. So the VL should be 8. |
llvm/test/Transforms/InterleavedAccess/RISCV/interleaved-accesses.ll | ||
---|---|---|
18 | Please make sure the tests can't be optimized to nothing. These shuffles aren't used so are allowed to be deleted. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
1201 | Em, is it enough to check bit size only? I think floating point elements will not produce new bit size. | |
1225 | Sorry, I should clang tidy every diff before uploading. | |
1227 | Em, I'm a little confused here. I understand RISCVTargetLowering::getContainerForFixedLengthVector is used in backend when SDAG is built as this method operates on MVT types. So if we just use opt in the middle-end, is this still needed? Further more, if something larger than LMUL=8 appears, could it be legalized by the backend? | |
llvm/test/Transforms/InterleavedAccess/RISCV/interleaved-accesses.ll | ||
9 | Fix this. It's my mistake. | |
18 | Could we guarantee this by carefully using the RUN instructions in the beginning of the test case? |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
1201 | You're creating a target specific intrinsic, like Intrinsic::riscv_vlseg2. If the floating point type isn't legal, the backend will error when trying to select the intrinsic. If its not legal and this code doesn't touch it, then the backend will be able to scalarize the loads and shuffles. | |
1227 | All conversions from a fixed vector type to a scalable vector must follow the VLEN passed on the command line in -riscv-v-vector-bits-min. The backend should not be splitting a scalable type if the fixed vector type was supposed to be legal type. | |
llvm/test/Transforms/InterleavedAccess/RISCV/interleaved-accesses.ll | ||
18 | Technically yes, but I wanted to see how this codegened in SelectionDAG and everything got deleted. So I'd rather have a more robust test. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
1201 | Thanks. I understand. | |
1227 | Yeah, you are right. It takes me some time to understand that. I think we need to make sure the target specific intrinsic is legal according to the VLEN passed on the command line in -riscv-v-vector-bits-min, so either fixed length vector or scalable vector need legality check (or conversion) for they might exceed the limit of max(LMUL)==8. It seems to be a little bit complicated. Or maybe we could fix this in another patch? | |
llvm/test/Transforms/InterleavedAccess/RISCV/interleaved-accesses.ll | ||
18 | I understand. Address comment. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
1227 | This needs to be handled in this patch. I don't want to think through issues that can arise by not honoring the VLEN correctly. It might be easier to add fixed vector instrinsics for segment load/store and handle the conversion from fixed vector to scalable vector inside of SelectionDAG where we already have helper functions for the conversion. This is what I did for the RISCVGatherScatterLowering pass which uses masked_strided_load and masked_strided_store intrinsics. |
Does this need to check if floating point elements are supported?