[PATCH] D124616: [TTI][X86] Fix splat-load cost when load+broadcast cannot be combined.

[Valeriy Dmitriev via Phabricator via llvm-commits]

vdmitrie added inline comments.


================
Comment at: llvm/lib/Target/X86/X86TargetTransformInfo.cpp:1616
+      bool LoadCanBeCombined =
+          L->hasOneUse() && isa<Instruction>(L->user_back());
+      if (ST->hasSSE3() && LoadCanBeCombined)
----------------
vporpo wrote:
> dmgreen wrote:
> > The user of an instruction should always be an instruction.
> > 
> > Just to make sure - it isn't a problem for the SLP vectorizer is it? That the load might have multiple uses, which we are turning into a splat?
> Oops, I will fix it.
> 
> Good point. I did not see any test failures, but I am pretty sure we won't get the right shuffle cost because we are querying the cost model before generating the vector code (which btw is a good reason for testing the actual cost values with something like https://reviews.llvm.org/D124802). 
> Hmm not sure what the best approach would be. Perhaps we could go through the users and count that we have at most one vector instruction. In this way SLP will still work as the users will be scalar and TTI still works with regular vector instructions. Any thoughts?
This new part of TTI interface actually lacks description. For broadcast kind it sounds like we better to pass just one load instruction which we want to broadcast rather than the whole set that form the splat (otherwise we indeed applying assumption wrt intended use of the interface). Since we are broadcasting a scalar load  its users are either scalar instructions or an insertelement.  What did you mean by vector instruction user?

During SLP cost modeling we have:

    %i1 = load double, double* %gep, align 8
    %i1 = load double, double* %gep, align 8
     fmul double %i1, ..
     fmul double %i1, ..
...
    . fadd double %i1, ...

which we may transform into 

  %i1 = load double, double* %gep, align 8
  %0 = insertelement <2 x double> poison, double %i1, i32 0
  %1 = insertelement <2 x double> %0, double %i1, i32 1
    fmul <2 x double> %1
   ...
     fadd double %i1, ...

i.e. we have two uses to form the splat and one extra user. You probably intended to check one user per lane (i.e. each use will form a lane in a vector). But that is not that straightforward when we for example trying to vectorize with 2 or more users of the splat.

If we end up vectorizing the tree
CG can generate either code like this:

   vmovsd   xmm1, mem64         ; load          (throughput 0.5)
   vmovddup xmm2, xmm1          ; register form  (throughput 1)
   vmulpd xmm2..

   vaddsd  xmm1

or like this
   vmovddup xmm1, mem64        ; load form  (throughput 0.5)
   vmulpd xmm1..

   vaddsd  xmm1                ; scalar user


Based on costs I tend to think CG should always prefer the second option (i.e. memory form)
CG may consider other factors than cost when life range between load and its scalar
users crosses block boundaries but since extract from vector register on x86 is essentially
free it is again makes memory form of movddup look more profitable.
We need somebody familiar with codegen to comment here.





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124616