[PATCH] D31961: DAGCombine: Combine shuffles of splat-shuffles

[Zvi Rackover via Phabricator via llvm-commits]

zvi added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14635-14642
+  for (unsigned i = 0; i < NumElts; ++i)
+    if (UserMask[i] < 0)
+      NewMask.push_back(Splat->getMaskElt(i));
+    else {
+      int NewIdx = Splat->getMaskElt(UserMask[i]);
+      IsSameMask &= NewIdx == Splat->getMaskElt(i);
+      NewMask.push_back(NewIdx);
----------------
spatel wrote:
> I'm scared that I'm not seeing all of the undef possibilities here (since I definitely missed them the first time!).
> 
> How about writing this to make the options more visible (not sure if it's more or less efficient), but I'd find this easier to read (warning: untested code):
>   // If the splat mask has no undefs, then return it as-is. 
>   // It doesn't matter if the user mask has undefs. We choose the splat mask element because it's efficient to return an existing splat node.
>   ArrayRef<int> SplatMask = Splat->getMask();
>   if (find(SplatMask, -1) == SplatMask.end())
>     return SDValue(Splat, 0);
>   
>   // The splat mask has undefs. If the user shuffle does not choose undef elements where the splat does not have undef elements, return the existing splat node.
>   if (all_of(UserMask, [](int Idx) { return Idx == -1 || Splat->getMaskElt(Idx) == Splat->getSplatIndex(); }))
>     return SDValue(Splat, 0); 
>     
>   // The splat mask has undefs, and the user shuffle chooses undef elements where the splat has defined elements. Create a more liberal splat mask (one with more undefs).
>    for (unsigned i = 0; i < NumElts; ++i)
>     // No need for "IsSameMask" here because we already handled any case where we wanted to return the existing node.
```
if (all_of(UserMask, [](int Idx) { return Idx == -1 || Splat->getMaskElt(Idx) == Splat->getSplatIndex(); }))
  return SDValue(Splat, 0);

```
This is not correct. For example UserMask = [0,2,u,u], SplatMask=[2,u,2,u]. It would be incorrect to return 'Splat' because it would result in an undef at element index=1 which the composition of masks does not yield.
But i will try to rewrite this code to follow the spirit of your suggestion: first try to identify cases where we can return the splat-shuffle itself, if not, build a new shuffle. Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D31961