[PATCH] D101109: [SLP]Improve multinode analysis.
Vasileios Porpodas via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 3 12:41:11 PST 2021
vporpo added inline comments.
================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:1458
+ I->getParent() != Parent) {
+ if (NumOpsWithSameOpcodeParent == 0) {
+ NumOpsWithSameOpcodeParent = 1;
----------------
Are you using `NumOpsWithSameOpcodeParent == 0` as a check for the first iteration ? Shouldn't you be using `!OpcodeI` intsead ?
I find this code a bit hard to follow, because I can't tell which of the `if` conditions are for checking for the first iteration and which ones are part of the heuristic. Should it be updating the `OpcodeI` and `Parent` only in the first iteration (like below), or should it be doing it whenever there is a mismatch?
```
if (auto *I = dyn_cast<Instruction>(OpData.V)) {
// First iteration
if (!OpcodeI) {
OpcodeI = I;
Parent = I->getParent();
}
// Mismatch
if (!getSameOpcode({OpcodeI, I}).getOpcode() || I->getParent() != Parent)
++NumOpsWithSameOpcodeParent;
else
NumOpsWithSameOpcodeParent = std::min(NumOpsWithSameOpcodeParent-1, 0);
}
```
Perhaps peeling the first iteration might make the code easier to follow?
================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:1459
+ if (NumOpsWithSameOpcodeParent == 0) {
+ NumOpsWithSameOpcodeParent = 1;
+ OpcodeI = I;
----------------
Why is `NumOpsWithSameOpcodeParent` set to 1 the first time a mismatch is found? Shouldn't it be set to 0 ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101109/new/
https://reviews.llvm.org/D101109
More information about the llvm-commits
mailing list