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
Unit Tests
| Time | Test | |
|---|---|---|
| 6,780 ms | x64 debian > libomp.tasking::omp50_taskwait_depend.c |
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 | ||
| 7700 ↗ | (On Diff #371782) | 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 | ||
|---|---|---|
| 7700 ↗ | (On Diff #371782) | These assertions fail on cases that masked loads do not. |
| llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | ||
|---|---|---|
| 7700 ↗ | (On Diff #371782) | 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 | ||
| 2774 ↗ | (On Diff #371966) | return Lo would simplify this flow a bit |
| 4344 ↗ | (On Diff #371966) | 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? |
| 5223 ↗ | (On Diff #371966) | 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.