[PATCH] D159205: [SLP]Try to vectorize scalars, being vectorized already, but do not need to be scheduled.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In D159205#4648813 <https://reviews.llvm.org/D159205#4648813>, @ABataev wrote:

> In D159205#4648767 <https://reviews.llvm.org/D159205#4648767>, @bgraur wrote:
>
>> In D159205#4648763 <https://reviews.llvm.org/D159205#4648763>, @ABataev wrote:
>>
>>> In D159205#4648753 <https://reviews.llvm.org/D159205#4648753>, @bgraur wrote:
>>>
>>>>> Is there something else above, like assertion message? It would really help more rather than the rest of the stack trace
>>>>
>>>> No, I've built the code with assertions enabled in the compiler and no assertion was triggered. The stack trace is all I get.
>>>
>>> Ok, I'll try to prepare a fix, but it would be good to see a reproducer.
>>
>> Working on the repro. Please CC me to the fix and I can check if it fixes the issue.
>
> I have a patch, could you try it?
>
>   diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
>   index 38ef4fa9464f..94abd8ac59da 100644
>   --- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
>   +++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
>   @@ -2916,8 +2916,11 @@ private:
>            const TreeEntry *TE = getTreeEntry(V);
>            assert((!TE || TE == Last || doesNotNeedToBeScheduled(V)) &&
>                   "Scalar already in tree!");
>   -        if (TE)
>   +        if (TE) {
>   +          if (TE != Last)
>   +            MultiNodeScalar.insert(V);
>              continue;
>   +        }
>            ScalarToTreeEntry[V] = Last;
>          }
>          // Update the scheduler bundle to point to this TreeEntry.
>   @@ -2976,6 +2979,9 @@ private:
>      /// Maps a specific scalar to its tree entry.
>      SmallDenseMap<Value *, TreeEntry *> ScalarToTreeEntry;
>   
>   +  /// List of scalar, used in several vectorize nodes.
>   +  SmallDenseSet<Value *> MultiNodeScalar;
>   +
>      /// Maps a value to the proposed vectorizable size.
>      SmallDenseMap<Value *, unsigned> InstrElementSize;
>   
>   @@ -9843,17 +9849,32 @@ Value *BoUpSLP::vectorizeOperand(TreeEntry *E, unsigned NodeIdx) {
>          S = getSameOpcode(*It, *TLI);
>      }
>      if (S.getOpcode()) {
>   -    if (TreeEntry *VE = getTreeEntry(S.OpValue);
>   -        VE && VE->isSame(VL) &&
>   -        (any_of(VE->UserTreeIndices,
>   -                [E, NodeIdx](const EdgeInfo &EI) {
>   -                  return EI.UserTE == E && EI.EdgeIdx == NodeIdx;
>   -                }) ||
>   -         any_of(VectorizableTree,
>   -                [E, NodeIdx, VE](const std::unique_ptr<TreeEntry> &TE) {
>   -                  return TE->isOperandGatherNode({E, NodeIdx}) &&
>   -                         VE->isSame(TE->Scalars);
>   -                }))) {
>   +    auto CheckSameVE = [&](const TreeEntry *VE) {
>   +      return VE->isSame(VL) &&
>   +             (any_of(VE->UserTreeIndices,
>   +                     [E, NodeIdx](const EdgeInfo &EI) {
>   +                       return EI.UserTE == E && EI.EdgeIdx == NodeIdx;
>   +                     }) ||
>   +              any_of(VectorizableTree,
>   +                     [E, NodeIdx, VE](const std::unique_ptr<TreeEntry> &TE) {
>   +                       return TE->isOperandGatherNode({E, NodeIdx}) &&
>   +                              VE->isSame(TE->Scalars);
>   +                     }));
>   +    };
>   +    TreeEntry *VE = getTreeEntry(S.OpValue);
>   +    bool IsSameVE = VE && CheckSameVE(VE);
>   +    if (!IsSameVE && MultiNodeScalar.contains(S.OpValue)) {
>   +      auto *I =
>   +          find_if(VectorizableTree, [&](const std::unique_ptr<TreeEntry> &TE) {
>   +            return TE->State != TreeEntry::NeedToGather && TE.get() != VE &&
>   +                   CheckSameVE(TE.get());
>   +          });
>   +      if (I != VectorizableTree.end()) {
>   +        VE = I->get();
>   +        IsSameVE = true;
>   +      }
>   +    }
>   +    if (IsSameVE) {
>          auto FinalShuffle = [&](Value *V, ArrayRef<int> Mask) {
>            ShuffleInstructionBuilder ShuffleBuilder(Builder, *this);
>            ShuffleBuilder.add(V, Mask);

I was able to prepare a reproducer for this, will fix it soon. Let me know if it fixes your issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159205