[PATCH] D135876: [InstCombine] Remove redundant splats in InstCombineVectorOps

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:2605
+Instruction *InstCombinerImpl::simplifyBinOpSplats(ShuffleVectorInst &SVI) {
+  if (!SVI.isZeroEltSplat())
+    return nullptr;
----------------
peterwaller-arm wrote:
> I'm seeing instcombine fire on this node:
> ```
> IC: Visiting:   %shuffle = shufflevector <2 x double> %sub, <2 x double> %sub, <2 x i32> <i32 2, i32 2>
> ```
> 
> Where I understand the intent is to only match the first element. The combine then replaces the input with:
> ```
> shufflevector <2 x double> %3, <2 x double> poison, <2 x i32> <i32 2, i32 2>
> ```
> 
> ... which is selecting elements from poison.
> 
> The reason the combine is firing is that `isZeroEltSplat` will return true if the mask is selecting the zeroth lane of any input operand.
> 
Normally, we wouldn't have to consider that case because it gets canonicalized (see around line 2696), but this transform can happen before shuffle mask canonicalization, so we'll need to check for a real zero mask. 

That check was included in an earlier draft, but I suggested `isZeroEltSplat` as a shortcut - oops. 

The header comments for that function clearly state that it works for the 0-elt of the 2nd shuffle operand, but I don't remember why I defined it that way...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135876