I've left mask registers to a future patch as we'll need
to convert them to full vectors, shuffle, and then truncate.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
llvm/utils/TableGen/CodeGenDAGPatterns.cpp | ||
---|---|---|
731 | This hack is to prevent tablegen from throwing a bunch of warnings about assuming a vector type is not scalable. This occurs because of my use of SDTCisSameSizeAs in the type profile for vrgather_vv_vl in tablegen. SmallSet defaults to std::less as its comparison function. But TypeSize doesn't implement operator< natively. But it appears to match to the operator< for uint64_t since there's a conversion operator to uint64_t in TypeSize. But that only works for fixed vectors. To workaround this, I explicitly split up the TypeSize into a std::pair. Adding an operator< to TypeSize didn't seem like a good idea. Another option might be to switch to SmallDenseSet and add a DenseMapInfo for TypeSize. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
1496 | I suppose we'll have to promote to i16 for this case? | |
llvm/utils/TableGen/CodeGenDAGPatterns.cpp | ||
731 | Hmm. I agree that operator< for TypeSize doesn't sound right. I can see the benefit of having a DenseMapInfo for TypeSize. I guess another option is to overload the comparison for this one case? That's quite short-sighted though. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
1496 | Yeah, but we don't really have a good way of know if it is needed without just always doing it or deciding based on whether -riscv-vector-bits-max is set and what its value is? We also can't promote to i16 without splitting for lmul=8. |
Use a custom comparator struct in tablegen. Thought about a DenseMap, but wasn't sure what to use for empty key and tombstone key.
Overall LGTM, waiting for @frasercrmck's comment.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
1496 | Agree, it's really a problem, unfortunately vrgatherei16 can not support i8m8 case directly. I think maybe compiler could emit a warning for non-workable case if user specific -riscv-vector-bits-max. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
1496 | Yeah, nothing really works well in this situation. It's just that with <vscale x 64 x i8> being legal it's not a problem for the future - it would be silently generating wrong code right now on many architectures. What if, for now, we decided not to support i8 unless -riscv-vector-bits-max was passed? | |
llvm/utils/TableGen/CodeGenDAGPatterns.cpp | ||
718 | I think this is an improvement over the std::pair. |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2662 | Is there an issue with using CONCAT_VECTORS? |
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2662 | We don’t seem to have implemented CONCAT_VECTORS for scalable vectors yet so it went to the stack. |
LGTM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp | ||
---|---|---|
2662 | Ah of course; we only added it for fixed-length vectors since it's common during legalization. |
clang-format not found in user's PATH; not linting file.