[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:1459
+            if (NumOpsWithSameOpcodeParent == 0) {
+              NumOpsWithSameOpcodeParent = 1;
+              OpcodeI = I;
----------------
vporpo wrote:
> ABataev wrote:
> > 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.
> Yes, it is increasing it, but shouldn't it be decreasing it instead (or letting it remain 0) ? This code block executes when there is a mismatch of opcode or parent (or if it is the first iteration), so shouldn't we be decreasing the value of`NumOpsWithSameOpcodeParent` (like in line 1463)?
> 
> What confuses me here is that `NumOpsWithSameOpcodeParent` looks like a normal counter that counts the opcode/parent matches. So I would expect it to increase by one if  the opcode/parents match (like what line 1466 does), and to decrease by one if there is a mismatch. But it seems to be more complicated than that: When it reaches 0 it foced to 1 even when there is an opcode mismatch. I find this a bit counter intuitive.
> 
> For example if we have mismatching opcodes in sequence, I would expect it to keep decreasing, or at least be capped to 0. But it seems like the value of  `NumOpsWithSameOpcodeParent` will be 0, then 1, then 0, then 1 like so:
> ```
> before the loop: 0
> iteration 1:  1 (because it was == 0)
> iteration 2:  0 (because of opcode mismatch)
> iteration 3:  1 (because it was == 0)
> ```
This is how the voting algorithm works. Here is described the main idea https://www.geeksforgeeks.org/boyer-moore-majority-voting-algorithm/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101109