Following the comment's thread of D117235, I added checks for the widening + splitting case, which also causes a split with one of the resulting vectors to be empty. Due to the same issues described in that same thread, the fixed-vectors-strided-store.ll test is missing the widening + splitting case, while the same case in the strided-vpload.ll test requires to manually split the loaded vector.
Details
Diff Detail
Event Timeline
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp | ||
---|---|---|
1891 | Does MemVT for a VP_STRIDED_LOAD ever mismatch with the element count of the result. This GetDependentSplitDestVTs is because of how we widen masked loads, but we aren't doing that same widening for strided loads. | |
1921 | Can the HiIsEmpty case happen? | |
1939 | Doesn't the offset need to include the stride which you can't do just like you can't for scalable vectors. |
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp | ||
---|---|---|
1895 | We're inside of type legalization, why wouldn't the stride type get legalized after the split? Though you wouldn't be able to use it in the ISD::MUL below if you didn't extend it first. |
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp | ||
---|---|---|
1891 | I am using GetDependentSplitDestVTs because I need the HiIsEmpty flag, I do not know if there is a better way to do it though. | |
1895 | I forgot to add the legalization steps for integer operands. I opened a new revision to address this: D123112 | |
1921 | I think that in the case that we have a widening followed by a split it may happen that one of the resulting vectors is empty, similar to what happens for VP_LOAD and VP_STORE. But it also may be that I am misunderstanding how the splitting for strided intrinsics should work. |
Changelog:
- Address some of the review comments
A new parent revision was added: now there is no need anymore to manually handle the stride operand expansion/truncation.
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp | ||
---|---|---|
1891 | I don't know what I was thinking before. The memory VT keeps the original type so I guess this is correct. |
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp | ||
---|---|---|
1936 | This will create a MachineMemOperand with size based on HiMemVT, but I don't think that's correct. We need UnknownSize. |
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp | ||
---|---|---|
1936 | In getStridedLoadVP the MachineMemOperand is created using MemoryLocation::UnknownSize, isn't this correct? |
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp | ||
---|---|---|
1936 | I didn't realize getStridedLoadVP did that. I think I noticed the inconsistency between store and load and thought there was something wrong. I think we should make them consistent. |
Does MemVT for a VP_STRIDED_LOAD ever mismatch with the element count of the result. This GetDependentSplitDestVTs is because of how we widen masked loads, but we aren't doing that same widening for strided loads.