[PATCH] D101109: [SLP]Improve multinode analysis.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:1458
+              I->getParent() != Parent) {
+            if (NumOpsWithSameOpcodeParent == 0) {
+              NumOpsWithSameOpcodeParent = 1;
----------------
vporpo wrote:
> 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?
This is again a kind of voting algorithm. This code works every time, we start voting on a value with the new opcode, not only on the first iteration. We just try to find the opcode with not less than NumOperands/2 number of occurrences here, if no such opcode - just choose any of them, there are no profitable elements.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:1459
+            if (NumOpsWithSameOpcodeParent == 0) {
+              NumOpsWithSameOpcodeParent = 1;
+              OpcodeI = I;
----------------
vporpo wrote:
> Why is `NumOpsWithSameOpcodeParent` set to 1 the first time a mismatch is found? Shouldn't it be set to 0 ?
It is a kind of increasing the counter for the first element in the sequence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101109