[PATCH] D93687: [VE] Extract & insert vector element isel

[Kazushi Marukawa via Phabricator via llvm-commits]

kaz7 added inline comments.


================
Comment at: llvm/lib/Target/VE/VEISelLowering.cpp:293
 #include "VVPNodes.def"
+
+    setOperationAction(ISD::INSERT_VECTOR_ELT, LegalVecVT, Legal);
----------------
The definition of BUILD_VECTOR action and other INSERT_VECTOR_ELT/EXTRACT_VECTOR_ELT actions are separated by VVP actions.


================
Comment at: llvm/lib/Target/VE/VEInstrPatternsVec.td:18
 
-multiclass vbrd_elem32<ValueType v32, ValueType s32, SDPatternOperator ImmOp, SDNodeXForm ImmCast, int SubRegIdx> {
+multiclass vbrd_elem32<ValueType v32, ValueType s32, SDPatternOperator ImmOp, SDNodeXForm ImmCast, SDNodeXForm SuperRegCast> {
   // VBRDil
----------------
Long lines.


================
Comment at: llvm/lib/Target/VE/VEInstrPatternsVec.td:27
+              (SuperRegCast $sy),
               i32:$vl)>;
 }
----------------
You can merge these three lines.


================
Comment at: llvm/lib/Target/VE/VEInstrPatternsVec.td:74
+  defm : vbrd_elem32<v32, s32, ImmOp, ImmCast, SuperRegCast>;
+  defm: extract_insert_elem32<v32, s32, SubRegCast, SuperRegCast>;
+}
----------------
`defm : extract...`?  Missing a space.


================
Comment at: llvm/test/CodeGen/VE/Vector/extract_elt.ll:6
+
+define fastcc i64 @extract_rr_v256i64(i32 %idx, <256 x i64> %v) {
+; CHECK-LABEL: extract_rr_v256i64:
----------------
It's better to use `i32 signext %idx` for arguments.


================
Comment at: llvm/test/CodeGen/VE/Vector/extract_elt.ll:35
+
+define fastcc i64 @extract_ri_v512i64(<512 x i64> %v) {
+; CHECK-LABEL: extract_ri_v512i64:
----------------
I simply don't understand why this won't cause errors...


================
Comment at: llvm/test/CodeGen/VE/Vector/extract_elt.ll:46
+
+define fastcc i32 @extract_rr_v256i32(i32 %idx, <256 x i32> %v) {
+; CHECK-LABEL: extract_rr_v256i32:
----------------
Same here.  `i32 signext/zeroext %idx`.


================
Comment at: llvm/test/CodeGen/VE/Vector/insert_elt.ll:7
+
+define fastcc <256 x i64> @insert_rr_v256i64(i32 %idx, i64 %s) {
+; CHECK-LABEL: insert_rr_v256i64:
----------------
`i32 signext/zeroext %idx`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93687/new/

https://reviews.llvm.org/D93687