[PATCH] D94708: [IR] Introduce llvm.experimental.vector.splice intrinsic

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:5524-5531
+  SDValue Chain = Expanded->getChain();
+  SDValue Ptr = Expanded->getOperand(1);
+
+  // Load the lo part of the spliced result
+  Lo = DAG.getLoad(LoVT, DL, Chain, Ptr, PtrInfo);
+  IncrementPointer(cast<MemSDNode>(Lo), LoVT, PtrInfo, Ptr);
+  // Load the hi part of the spliced result
----------------
is there a way to split the load using existing code?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:10911
+
+  uint64_t Idx = (NumElts + (Imm % NumElts)) % NumElts;
+
----------------
just `Imm % NumElts`; ?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:8657
+                              VT.getStoreSize().getKnownMinSize()));
+      TrailingBytes = DAG.getNode(ISD::UMIN, DL, PtrVT, TrailingBytes, VLBytes);
+    }
----------------
You'll need to do a similar clamping for Imm > 0 (or actually, Imm > MinKnownVL) , to avoid loading beyond the allocated stack object.


================
Comment at: llvm/test/CodeGen/AArch64/named-vector-shuffles-neon.ll:19
+
+define <8 x i16> @splice_v8i16_idx(<8 x i16> %a, <8 x i16> %b) #0 {
+; CHECK-LABEL: splice_v8i16_idx:
----------------
nit: not sure if there is a lot of value to do this for each possible element-type, if you do two (i8 and some other type) that may be sufficient.


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

https://reviews.llvm.org/D94708