Isel and tests for extract_vector_elt and insert_vector_elt.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
- Fixed custom lowering code for packed vectors and updated tests.
- Removed unrelated BUILD_VECTOR custom lowering to insert/extract elt.
llvm/lib/Target/VE/VEISelLowering.cpp | ||
---|---|---|
307 | The definition of BUILD_VECTOR action and other INSERT_VECTOR_ELT/EXTRACT_VECTOR_ELT actions are separated by VVP actions. | |
llvm/lib/Target/VE/VEInstrPatternsVec.td | ||
18–19 | Long lines. | |
26 | You can merge these three lines. | |
74 | defm : extract...? Missing a space. | |
llvm/test/CodeGen/VE/Vector/extract_elt.ll | ||
7 | It's better to use i32 signext %idx for arguments. | |
36 | I simply don't understand why this won't cause errors... | |
47 | Same here. i32 signext/zeroext %idx. | |
llvm/test/CodeGen/VE/Vector/insert_elt.ll | ||
8 | i32 signext/zeroext %idx. |
NFC
- Use signext for arguments in tests.
- No more than 80 chars in VEInstrPatternVec.td file.
llvm/lib/Target/VE/VEISelLowering.cpp | ||
---|---|---|
307 | You mean that the setOperationAction calls for VVP are not at the top or bottom of the initVPUActions function? | |
llvm/lib/Target/VE/VEInstrPatternsVec.td | ||
26 | Great! I suppose you mean the VBRDrl and VBRDil patterns. How do i do this? There would need to be a noop replacement for the ImmOp Operand.. | |
llvm/test/CodeGen/VE/Vector/extract_elt.ll | ||
36 | Isel legalizes this into two v256i64 arguments, which are mapped to v0 and v1 |
llvm/lib/Target/VE/VEISelLowering.cpp | ||
---|---|---|
307 | Yes. If you have a certain reason to do that, it's fine. But, at least, I would like to understand it since I may change codes also. If you are doing without any reason, I would like to ask you to organize changes in maybe next patch. | |
llvm/lib/Target/VE/VEInstrPatternsVec.td | ||
26 | No. What I said is merge three lines into one line like (VBRDrl (SuperRegCast $sy) i32:$vl)>; Doing that organize vbrd_elm32 and vbrd_elm64 consitently and make review easy. | |
llvm/test/CodeGen/VE/Vector/extract_elt.ll | ||
36 | I see. I was afraiding that compiler is generating wrong code without any warning/errors. Now, I've understood why lvs uses %v1(116) as its operands. Thanks. | |
86 | i32 signext %idx | |
182 | i32 signext %idx | |
llvm/test/CodeGen/VE/Vector/insert_elt.ll | ||
47 | i32 signext %s, or i32 zeroext %s to avoid and. | |
57 | same here. | |
67 | same here. | |
78 | same here. | |
92 | same here. |
A few more modificating candidates to make code better.
llvm/lib/Target/VE/VEISelLowering.cpp | ||
---|---|---|
2714 | Op.getValueType() returns EVT. I guess using Op.getSimpleValueType() is better for this. | |
2717 | Op.getSimpleValueType() | |
2744 | Idx.getSimpleValueType() | |
2746 | Val.getSimpleValueType() | |
2748 | Val.getSimpleValueType() |
getSimpleValueType()
llvm/lib/Target/VE/VEISelLowering.cpp | ||
---|---|---|
307 | Ok. This is now structured as follows: |
- Rebased onto main.
- insert_vector_elt tests trigger vbrd code path because lowering of build_vector to insert_vector_elt is gone (D93759).
clang-tidy: warning: invalid case style for function 'lowerEXTRACT_VECTOR_ELT' [readability-identifier-naming]
not useful