[SystemZ][TTI]Wrong costs for insertelement with even/odd indices

[alexey-bataev]

Looks like there is bug in SystemZTargetTransformInfo.cpp, SystemZTTIImpl::getVectorInstrCost:

if (Val->isIntOrIntVectorTy(BitWidth: 64))
  return ((Index % 2 == 0) ? 1 : 0);

It returns cost 1 for insertion into lane 0 and cost 0 for insertion into lane 1, though looks like SystemZTTIImpl::getScalarizationOverhead implements different logic

[alexey-bataev]

@uweigand

[uweigand]

As the comment says:

    // vlvgp will insert two grs into a vector register, so count half the
    // number of instructions as an estimate when we don't have the full
    // picture (as in getScalarizationOverhead()).

we have a single instruction that performs two inserts (i.e. load a 16-byte vector from 2 8-byte GPRs) at one time. Counting one insert as 1 instruction and the other as 0 instructions is an attempt to model this, which is of course not always correct.

@JonPsson1 can you have a look at #135245 where this came up?

[JonPsson1]

Yes, SystemZTTIImpl::getVectorInstrCost() needs to do an estimation based on the the type and the index per the comment. In getScalarizationOverhead(), the vector elements are available to inspect so it makes a better effort.

The test that you changed has a comment right above it:

; This should *not* be vectorized as the insertion into the vector isn't free,                                                                                               
; which is recognized in SystemZTTImpl::getScalarizationOverhead().   

I worked on the SLPVectorizer last year with #112491 (you approved it) which aimed to improve this area. IIRC, the SLPVectorizer then started passing VL to getScalarizationOverhead and the test was updated with it, but now it has been changed back to being vectorized.

This is how the test now has changed (vectorized to the right, as with your latest change):

SLP: Current total cost = -1                    SLP: Current total cost = -1
SLP: Adding cost 1 for bundle Idx: 1, n=2 [i6 | SLP: Adding cost 0 for bundle Idx: 1, n=2 [i6
SLP: Current total cost = 0                   | SLP: Current total cost = -1
                                              > SLP: Found cost = -1 for VF=2
                                              > SLP: Decided to vectorize cost = -1


define void @fun2(ptr %0, ptr %Dst) #0 {        define void @fun2(ptr %0, ptr %Dst) #0 {
  %2 = load i64, ptr %0, align 8                  %2 = load i64, ptr %0, align 8
  %3 = icmp eq i64 %2, 0                          %3 = icmp eq i64 %2, 0
  br i1 %3, label %4, label %5                    br i1 %3, label %4, label %5

4:                                              4:                                           
  ret void                                        ret void

5:                                              5:                                           
  %6 = getelementptr i8, ptr %Dst, i64 24     |   %6 = getelementptr i8, ptr %Dst, i64 16
  store i64 %2, ptr %6, align 8               |   %7 = insertelement <2 x i64> <i64 0, i64 po
  %7 = getelementptr i8, ptr %Dst, i64 16     |   store <2 x i64> %7, ptr %6, align 8
  store i64 0, ptr %7, align 8                <
  br label %4                                     br label %4
}                                               }


fun2:                                   # @fu   fun2:                                   # @fu
        .cfi_startproc                                  .cfi_startproc
# %bb.0:                                        # %bb.0:
        ltg     %r0, 0(%r2)                             ltg     %r0, 0(%r2)
        ber     %r14                                    ber     %r14
.LBB0_1:                                        .LBB0_1:
        stg     %r0, 24(%r3)                  |         lghi    %r1, 0
        mvghi   16(%r3), 0                    |         vlvgp   %v0, %r1, %r0
                                              >         vst     %v0, 16(%r3), 3
        br      %r14                                    br      %r14

So it seems that now again SLPVectorizer disregards the insertion cost and thinks that it is now doing a single vector store instead of two scalar ones, with a -1 total cost. This is obviosly wrong. At best the immediate load could be ignored and a cost of 0 (no vectorization) would result.

I think the idea so far on SystemZ is that we keep these stores separate unless there is a good reason (such as a free vector element load into the stored vector). In this example the store of immediate can be done without a separate lghi. In any case, we shouldn't change this behavior without careful performance evaluation.

Could you explain more what your latest change does and why this test is vectorized now?

[alexey-bataev]

The cost of the %7 = insertelement <2 x i64> <i64 0, i64 po is now 0 because TTI assumes that insertion into lane 1 is free

[alexey-bataev]

Changed the cost estimation of the insertion into non-poison value. Originally, getScalarizationOverhead was used, but it does not support non-poison source. So, had to switch to getVectorInstructionCost to estimate the cost of insertion into non-poison source

[JonPsson1]

ok, so we added the VL argument to getScalarizationOverhead() for this purpose in the first place. Is there any way you could pass this to getVectorInstructionCost()? Or could getScalarizationOverhead() be fixed to also handle this case?

[alexey-bataev]

VL is just a list of scalars to be inserted, it does not define the source vector value. getVectorInstructionCost has a parameter Op0, which specifies the source vector