[PATCH] D125546: [RISCV] Use tail agnostic if inserting subvector/element at the end of the vector.

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:4429
+  int PolicyVal = RISCVII::TAIL_UNDISTURBED_MASK_UNDISTURBED;
+  // We could use tail agnostic if it's inserting to the latest of a vector.
+  if (VecVT.isFixedLengthVector() && isa<ConstantSDNode>(Idx) &&
----------------
rogfer01 wrote:
> I'd use "at the end of the vector" (I think "latest" is about time)
+1


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:5360
+    if (VecVT.isFixedLengthVector() &&
+        (OrigIdx + SubVecVT.getVectorNumElements() + 1) ==
+            VecVT.getVectorNumElements())
----------------
rogfer01 wrote:
> I'm a bit confused here, probably I misunderstood what we mean by the end of the vector.
> 
> In the following testcase taken from `fixed-vectors-fp-shuffles.ll`
> 
> 
> ```lang=llvm
> define void @insert_v32i1_v8i1_16(<32 x i1>* %vp, <8 x i1>* %svp) #0 {
>   %v = load <32 x i1>, <32 x i1>* %vp, align 4
>   %sv = load <8 x i1>, <8 x i1>* %svp, align 1
>   %c = call <32 x i1> @llvm.experimental.vector.insert.v32i1.v8i1(<32 x i1> %v, <8 x i1> %sv, i64 16)
>   store <32 x i1> %c, <32 x i1>* %vp, align 4
>   ret void
> }
> 
> declare <32 x i1> @llvm.experimental.vector.insert.v32i1.v8i1(<32 x i1> %v, <8 x i1> %sv, i64 %idx)
> ```
> 
> Once the mask types are promoted to integer types we have: `OrigIdx == 2`, `VecVT == v4i8`, and `SubVecVT == v1i8`. My impression is that the end of the vector in this case would be when `OrigIdx == 3`. Does this align with your expectation or my notion of "end" is incorrect?
> 
> The condition check does `2 + 1 + 1 == 4` so we make this tail agnostic. Perhaps, not sure, the last `+ 1` is not needed (it was needed for the insert element case because you were checking the index in the vector).
I think you're right, Roger. Even if you're inserting 8 elements into an 8-element vector at position zero, we'd want that as tail agnostic, right? Currently we'd miss that as `0 + 8 + 1 != 8`. (I don't know if that's technically legal with `INSERT_SUBVECTOR` but I think the maths works out the same)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125546