This patch introduces two new experimental IR intrinsics and SDAG nodes to represent vector strided loads and stores.
Details
Diff Detail
Unit Tests
Time | Test | |
---|---|---|
50 ms | x64 debian > LLVM.Bindings/Go::go.test |
Event Timeline
llvm/include/llvm/IR/VPIntrinsics.def | ||
---|---|---|
217 | Is this an existing mistake in the documentation? Can you just push this as an NFC commit? No need to bury it in this patch. | |
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp | ||
7969 | Since this accesses multiple disjoint memory locations, the Size is not MemVT.getStoreSize(). It probably needs to always be unknown. I guess it would be if the stride were statically known to be 0, but not sure it is worth special casing that. | |
8102 | Same as load, the Size can't be based on the store type. | |
8158 | Same | |
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | ||
7449 | This Size doesn't make sense. I think it needs to be unknown. | |
7470 | Same | |
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h | ||
574 | Can these be SmallVectorImpl<SDValue>? |
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | ||
---|---|---|
7437 | Does anyone know if we should be passing AAInfo here? (Non-stride) VPLoad doesn't do that, so we can fix both in a later change if necessary. |
ChangeLog:
- add documentation for the new intrinsics
- use MemoryLocation::UnknownSize
- use SmallVectorImpl<SDValue> in functions signatures
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h | ||
---|---|---|
574 | I'll fix visitVPLoadGather and visitVPStoreScatter above in a later change. |
llvm/lib/IR/IntrinsicInst.cpp | ||
---|---|---|
495 | This will require an explicit return type because of opaque pointers, as in 6213f1dd03e27fd2483f8ac775a346de8e873573 |
Changelog:
- use explicit return type in Intrinsic::getDeclaration() for experimental_vp_strided_load
- get first argument type from first operand in Intrinsic::getDeclaration() for experimental_vp_strided_store
llvm/docs/LangRef.rst | ||
---|---|---|
19747 | I think mentioning that the align parameter can be used (and correctly handling that further down in SelectionDAG) would be good. |
Changelog:
- Mention that the align parameter attribute can be provided for the base pointer operands
llvm/docs/LangRef.rst | ||
---|---|---|
19735 | Reflecting on how the strided versions work, I was thinking if, semantically speaking, it would be better to define the pointer operand as a pointer to the scalar element type of the data/return operand instead of a pointer to a vector (this is only relevant while opaque pointers are not the default) |
llvm/docs/LangRef.rst | ||
---|---|---|
19735 | Agreed. Typed pointers will go away but we shouldn't be sloppy here. |
Changelog:
- Changed the pointer operand type to match the scalar elements type of the data/return operand
llvm/docs/LangRef.rst | ||
---|---|---|
19763 | The stride can have a different number of bits than the ptr. |
Reflecting on how the strided versions work, I was thinking if, semantically speaking, it would be better to define the pointer operand as a pointer to the scalar element type of the data/return operand instead of a pointer to a vector (this is only relevant while opaque pointers are not the default)