This modifies it to work on both scalable and fixed vectors
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2953 | Personally, I'd leave this duplicated in each caller. Keeping the helper as an unconditionally successful routine seems worth the very minor code duplication. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
3143 | Can this nest in the if (ContainerVT.isFixedLengthVector()) earlier. The source and destination should alway be both fixed or both scalable. | |
3147 | To do my above suggestion, this would need to use ContainerVT to keep SrcContainerVT isolated to the body of the if (ContainerVT.isFixedLengthVector()) |
LGTM w/comment applied.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
3483 | There's no point in moving this check to the caller as the isDeinterleaveShuffle routine is only being used here. (Unless maybe you have another patch in mind, but if so, the change should probably be it's own NFC with that explained.) |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
3483 | Yeah, IMO checking if the type is widenable is orthogonal to checking if a vector shuffle is a deinterleave. Splitting it out lets us reuse it for combining deinterleaves of loads of fixed vectors |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
3483 | Please revert to the old form of the check, and we can discuss this change in the DAG combine change which uses it. |
Personally, I'd leave this duplicated in each caller. Keeping the helper as an unconditionally successful routine seems worth the very minor code duplication.