This patch adds support for integer promotion for VP_LOAD and VP_STORE
by legalizing to extending and truncating forms of each, respectively.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp | ||
---|---|---|
769 | Should we choose the extend type from N if it was already an extending VP load? Similar to PromoteIntRes_LOAD. |
Yes you should be able to test this with RISCV. Anything that's not i1/i8/i16/i32 but is less than i64 will need promoting. Also see D108288 for examples.
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp | ||
---|---|---|
1973 | I don't think we expect EVL to need promoting - do we? It should always be a legal target-specific type, reported by TLI.getVPExplicitVectorLengthTy(). I don't know how well we're asserting on that right now. |
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp | ||
---|---|---|
1976 | GetPromotedInteger -> ZExtPromotedInteger. We need to force the upper bits to 0. | |
1977 | 5 -> 6 if I've counted the number of operands correctly. | |
1985 | Should we assert that the store isn't indexed since this call would be incorrect if it was? | |
1996 | GetPromotedInteger -> ZExtPromotedInteger. |
Added more legalization to this patch rather than putting it into another patch. Added another form of getLoadVP that was needed by the legalization. Expanded tests to cover vector splitting.
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp | ||
---|---|---|
1985 | Why does this use isIndexed() but load uses N->getAddressingMode() == ISD::UNINDEXED. Are the interfaces for checking indexed different for loads and stores? | |
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | ||
7696–7697 | Why should this be different than the assert in getLoad? |
llvm/test/CodeGen/Generic/fixed-vector-vp-mem-legalization.ll | ||
---|---|---|
2 ↗ | (On Diff #371791) | We shouldn't put RISC-V tests in Generic unless we know the target is built. So either this needs to use REQUIRES: or we put it in CodeGen/RISCV. I prefer the latter as that's where I've seen all other target-specific legalization tests go. |
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | ||
---|---|---|
7696–7697 | These assertions fail on cases that masked loads do not. |
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | ||
---|---|---|
7696–7697 | But why? Seems weird to have an extending load that doesn't extend. |
@hussainjk are you still working on this or are you taking over, @bmahjour? I might be able to find time if you're unavailable.
llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp | ||
---|---|---|
1994 | Again, I'm not sure whether we ever need to promote the length - it's always a target-specific legal integer type, isn't it? | |
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp | ||
2744 | return Lo would simplify this flow a bit | |
4388 | true here is FillWithZeroes - I don't think we need to do this for VP widening, since we're passing the original length along, right? | |
5298 | same comment about FillWithZeroes |
@hussainjk are you still working on this or are you taking over, @bmahjour? I might be able to find time if you're unavailable.
@frasercrmck, Hussain is not working on this any more. Please feel free to commandeer this revision. Thanks for your help!
- rebase
- drop widening and splitting (committed separately in D117235)
Note that while the legalized nodes are correct, the generated code we're
testing is *not*; RISCV ignores extending and truncating operations.
Should we choose the extend type from N if it was already an extending VP load? Similar to PromoteIntRes_LOAD.