[PATCH] D41948: [SLP] Fix vectorization for tree with trunc to minimum required bit width.

[Matthew Simpson via Phabricator via llvm-commits]

mssimpso added inline comments.


================
Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:4023-4025
+    // Do not include top zext/sext/trunc operations to those to be demoted, it
+    // produces noise cast<vect>, trunc <vect>, exctract <vect>, cast <extract>
+    // sequence.
----------------
ABataev wrote:
> mssimpso wrote:
> > What do you mean by "top" here?  Do you just mean that the initial Root should not be a cast? If so, would it be easier to check that here, rather than passing the flag in collectValuesToDemote?
> Yes, it is about initial roots. The main problem here is that these roots still must be analyzeв though not considered as candidates to demote.
> Meanwhile, I have a question for you. Why you try to generate sequence `cast ( extractelement(<resulting_vector>))` instead of `extractelement(cast(<resulting_vector>))`? It produces `2 * n` instructions, while the second approach will produce `n + 1` instructions.
I honestly don't recall what the reason was. It probably had something to do with the way InstCombine wanted to rewrite the expression, given the trunc/ext sequence.

I think what we should really try to do is make the type truncating code in InstCombine (EvaluateInDifferentType) a utility that we can use within the vectorizers. The trunc/ext trick is very fragile. We use it here in SLP,and in two separate places in LV to type-shrink reductions and other instructions.

I'll finish taking a look at the rest of the patch shortly.


Repository:
  rL LLVM

https://reviews.llvm.org/D41948