[PATCH] D141778: [DAGCombiner][X86] `mergeConsecutiveStores()`: support merging splat-stores of the same value

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/test/CodeGen/X86/legalize-shl-vec.ll:247-252
+; X64-NEXT:    movq %r8, %xmm0
+; X64-NEXT:    pshufd {{.*#+}} xmm0 = xmm0[0,1,0,1]
+; X64-NEXT:    psrad $31, %xmm0
+; X64-NEXT:    pshufd {{.*#+}} xmm0 = xmm0[1,1,3,3]
+; X64-NEXT:    movdqa %xmm0, 16(%rdi)
+; X64-NEXT:    movdqa %xmm0, (%rdi)
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > pengfei wrote:
> > > Looks like regression here.
> > We've traded 4 scalar stores to two vector stores + GPR->XMM xfer + two shuffles + shift.
> > I wouldn't say it's an obvious regression, since we get less contention in CPU store unit,
> > but it's not really an improvement, yes.
> > 
> > Can you help spot issues in the tests in `elementwise-store-of-scalar-splat.ll` / `subvectorwise-store-of-vector-splat.ll`?
> > If those do not have regressions, then we need to restrict some other fold.
> This is an unrelated shuffle combining issue
> ```
>         movq    %r8, %xmm0
>         pshufd  $68, %xmm0, %xmm0               # xmm0 = xmm0[0,1,0,1]
>         psrad   $31, %xmm0
>         pshufd  $245, %xmm0, %xmm0              # xmm0 = xmm0[1,1,3,3]
> ```
> What we want here, is to splat the sign bit of a 64-bit scalar to the entire XMM.
> This should just be: (note the decoded shuffle mask)
> ```
>         movq    %r8, %xmm0
>         pshufd  $<???>, %xmm0, %xmm0            # xmm0 = xmm0[1,1,1,1]
>         psrad   $31, %xmm0
> 
> ```
https://reviews.llvm.org/D141806 will address this.
Does anyone see any other regressions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141778