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

[Bin Cheng via Phabricator via llvm-commits]

bin.cheng-ali added inline comments.


================
Comment at: llvm/docs/LangRef.rst:16285
+``concat(%vec1, %vec2)`` starting at index ``imm``, whereas for a negative
+immediate, it extracts ``imm`` trailing elements from the first vector, and the
+remaining elements from ``%vec2``.
----------------
One nit.  Given ``imm`` is negative here, should this be?
"it extracts ``-imm`` trailing elements from the first vector, ..."


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:10911
+
+  uint64_t Idx = (NumElts + (Imm % NumElts)) % NumElts;
+
----------------
c-rhodes wrote:
> sdesmalen wrote:
> > just `Imm % NumElts`; ?
> > just `Imm % NumElts`; ?
> 
> I tried that at first but found the behaviour isn't correct for negative immediates, for example a trailing element count of `-15` and 16 elements `-15 % 16 = -15`, so end up with this mask: `[-15,-14,-13,-12,-11,-10,-9,-8,-7,-6,-5,-4,-3,-2,-1,0]`. From what I read the sign of the modulo result is implementation defined if one or both of the operands are are negative.
Then `(NumElts + Imm) % NumElts` ?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:8623
+  //    Ptr = Ptr + (Imm * sizeof(VT.Elt))
+  //  Res = Load Ptr
+
----------------
Sorry for may stupid question, this stores two vector register and load in the middle, does Endingness matters here?  How does LLVM make sure correct sequence of elements is loaded?  Thanks in advance.


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

https://reviews.llvm.org/D94708