[PATCH] D144689: [SLP]Improve handling gathers/buildvectors with undefs.

[Valeriy Dmitriev via Phabricator via llvm-commits]

vdmitrie added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:1449
         // Undefs are always profitable for extractelements.
         if (isa<UndefValue>(V2))
+          return (isa<PoisonValue>(V2) || isUndefVector(EV1).all())
----------------
This needs a bit of explanation (a comment). 


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4188
+      if (It == TE.Scalars.begin())
+        return {};
+      auto *Ty = FixedVectorType::get(TE.Scalars.front()->getType(), Sz);
----------------
Could you please clarify the difference between returning empty container vs std::nullopt?
The description comment for getReorderingData method does not mention this distinction.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4192-4206
+        Order[std::distance(TE.Scalars.begin(), It)] = 0;
+        fixupOrderingIndices(Order);
+        SmallVector<int> Mask;
+        inversePermutation(Order, Mask);
+        if (TTI->getVectorInstrCost(Instruction::InsertElement, Ty,
+                                    TTI::TCK_RecipThroughput, 0,
+                                    PoisonValue::get(Ty), *It) +
----------------
Just for the sake of better readability can you rearrange the code to add few variables and break down into pieces that jumbo if condition, please?

Like for example here:

`        unsigned Idx = std::distance(TE.Scalars.begin(), It);
         Order[Idx] = 0;
`
...
`        InstructionCost PermuteCost =
            TopToBottom
                ? 0
                : TTI->getShuffleCost(TTI::SK_PermuteSingleSrc, Ty, Mask);
        InstructionCost InsertFirstCost = TTI->getVectorInstrCost(
            Instruction::InsertElement, Ty, TTI::TCK_RecipThroughput, 0,
            PoisonValue::get(Ty), *It);
        InstructionCost InsertIdxCost = TTI->getVectorInstrCost(
            Instruction::InsertElement, Ty, TTI::TCK_RecipThroughput, Idx,
            PoisonValue::get(Ty), *It);
        if (InsertFirstCost + PermuteCost < InsertIdxCost)
          return Order;
` 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144689