[llvm] [SLP] Check for extracts, being replaced by original scalars, for user nodes (PR #149572)

[Gaëtan Bossu via llvm-commits]

https://github.com/gbossu commented:

Sorry for getting back to this only now. I left a couple more comments, but I don't feel comfortable approving that change, as I think I'm still missing something.

AFAIU, we are trying to find out if it is worth vectorising instructions when some of them might need to be kept as scalars as well due to scalar users. That is why we first check if the user `TreeEntry` will have some of its instructions kept as scalars:
 - If there are none, then we can go ahead and vectorise `VL`
 - But if there are some, then we also need the operands of those scalar users to be scalarised. On top of that, we would need to gather all the instructions into a vector so that they can be used by the user `TreeEntry` as well. I think this is what you're checking with `ExtractCost < UserScalarsCost + ScalarsCost`

I still think the cost checks are too "local" to really answer the question "Is this worth vectorising if some lanes need to be kept as scalars?". It's not clear to me if looking at a single user `TreeEntry` is a good enough heuristic. To me this should be done when the whole tree is built. At this point, we can accurately compute the cost of keeping some lanes of a vectorised instruction as scalars because we also know which operands need to be scalarised as well as vectorised. If a `TreeEntry` turns out to be too costly because it is both partially scalarised and vectorised, then we can replace it with a Gather node, and essentially prune the tree.

I'll let the usual reviewers do a more thorough review, I don't understand enough about the patch (and the SLPVectorizer in general) to give an approval.

https://github.com/llvm/llvm-project/pull/149572