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 | ||
|---|---|---|
| 748 | 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 | ||
|---|---|---|
| 1921 | 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 | ||
|---|---|---|
| 1924 | GetPromotedInteger -> ZExtPromotedInteger. We need to force the upper bits to 0. | |
| 1925 | 5 -> 6 if I've counted the number of operands correctly. | |
| 1933 | Should we assert that the store isn't indexed since this call would be incorrect if it was? | |
| 1944 | 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 | ||
|---|---|---|
| 1933 | 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 | ||
| 7700 | Why should this be different than the assert in getLoad? | |
| llvm/test/CodeGen/Generic/fixed-vector-vp-mem-legalization.ll | ||
|---|---|---|
| 3 | 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 | ||
|---|---|---|
| 7700 | These assertions fail on cases that masked loads do not. | |
| llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | ||
|---|---|---|
| 7700 | 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 | ||
|---|---|---|
| 1942 | 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 | ||
| 2774 | return Lo would simplify this flow a bit | |
| 4344 | 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? | |
| 5225 | 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.