[PATCH] D149627: [NFC][SLP] Cleanup: Simplify traversal loop in SLPVectorizerPass::vectorizeHorReduction().

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:13985
 /// \Returns the new root if found, which may be nullptr if not an instruction.
-static Instruction *tryGetScondaryReductionRoot(PHINode *Phi,
+static Instruction *tryGetSecondaryReductionRoot(PHINode *Phi,
                                                 Instruction *Root) {
----------------
1. Make a separate NFC patch.
2. Format, the second parameter is not aligned


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:14027-14029
+  bool TryOperandsAsNewSeeds = P != nullptr;
   if (!isa<BinaryOperator>(Root))
+    TryOperandsAsNewSeeds = false;
----------------
bool TryOperandsAsNewSeeds = P && isa<BinaryOperator>(Root);


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:14035-14042
+  Instruction *NewRoot = nullptr;
+  if (isReductionCandidate(Root)) {
+    assert((!TryOperandsAsNewSeeds || is_contained(P->operands(), Root)) &&
+           "Phi needs to use the binary operator");
+    if (TryOperandsAsNewSeeds &&
+        HorizontalReduction::getRdxKind(Root) != RecurKind::None)
+      NewRoot = tryGetSecondaryReductionRoot(P, Root);
----------------
```
if (P && isa<BinaryOperator>(Root) && HorizontalReduction::getRdxKind(Root) != RecurKind::None)
  Root = tryGetSecondaryReductionRoot(P, Root);
```


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:14071
+    Value *VectorizedV = nullptr;
+    if (!R.isAnalyzedReductionRoot(Inst) && isReductionCandidate(Inst)) {
+      HorizontalReduction HorRdx;
----------------
What if Inst is one of the P operands here, but not Root?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:14086
+      Instruction *FutureSeed = Inst;
+      if (TryOperandsAsNewSeeds && Inst == Root) {
+        FutureSeed = getNonPhiOperand(Root, P);
----------------
I think this is not quite same as before, it should not be a Root, it may be another P operand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149627