[PATCH] D149627: [NFC][SLP] Cleanup: Simplify traversal loop in SLPVectorizerPass::vectorizeHorReduction().
Vasileios Porpodas via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 2 11:13:32 PDT 2023
vporpo added inline comments.
================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:14008-14010
+ Instruction *Op = dyn_cast<Instruction>(Op0);
+ if (Op == Phi)
+ Op = dyn_cast<Instruction>(Op1);
----------------
ABataev wrote:
> ABataev wrote:
> > `auto *Op`
> `return dyn_cast<Instruction>(Op0 == Phi ? Op1 : Op0);`
Phi and Op are already Instruction * so I will use this instead: `return Op == Phi ? dyn_cast<Instruction>(Val: Op1) : Op; `
================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:14080
+ Instruction *FutureSeed = Inst;
+ if (TryOperandsAsNewSeeds && Inst == Root) {
+ FutureSeed = getNonPhiOperand(Root, P);
----------------
ABataev wrote:
> The original code does not compare with Root, it compares with P. What if Inst is an instruction with P operand, but not Root?
`P` is set to null after visiting the root node (lines 14060, 14075 and 14081). So the code will enter this `if` only if `Inst` is the root node. If it is any other instruction it won't enter.
================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:14083
+ if (!FutureSeed)
+ break;
}
----------------
ABataev wrote:
> Why break here?
This can only happen if Inst is the root node and at this point `Stack` is empty so it effectively breaks out of the loop, so it is better to be explicit about it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149627/new/
https://reviews.llvm.org/D149627
More information about the llvm-commits
mailing list