[VPlan] Support to widen select intructions in VPlan native path

[Kazhuu]

Summary

Add support to widen select instructions in VPlan native path by using a correct recipe when such instructions are encountered. This is already used by inner loop vectorizer.

Previously select instructions get handled by the wrong recipe and resulted in unreachable instruction errors like this one: https://bugs.llvm.org/show_bug.cgi?id=48139.

[Kazhuu]

Fix failing unit tests.

[fhahn]

Thanks for the patch! I think this can be split into 2 indepdennt patches (one for selects, one for calls). Also, please add new minimal test cases for both the call & select cases.

[Kazhuu]

Rebase with latest changes, add support for select instruction widening only in this patch and add test case for it. Add call instruction widening as a separate patch later.

[Kazhuu]

Call instruction widening as a separate patch here: https://reviews.llvm.org/D97278

[fhahn]

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

20

We only need SE, can you pass that directly?

75

I think we should test all possible for loop invariant conditions (in inner & outer loop).

[Kazhuu]

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

75

Can you elaborate a bit what you mean? Like test that is the condition invariant for all inner and outer loops?

[Kazhuu]

Removed use of PredicatedScalarEvolution and use ScalaEvolution instead to clean up diff.

[Kazhuu]

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

20

Thanks for the comment! I didn't pay attention enough for this. Removing PSE also cleaned up code in tests as well.

[fhahn]

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

75

Currently the condition of the select in the inner loop is invariant (because it's a function argument). We should have tests with conditions that are invariant only in parts of the loop (e.g. a select with a condition that depends on inner & outer IVs in the inner loop, a select with a condition that depends on the outer IV in the inner loop).

[Kazhuu]

Add test cases for different select conditions and clean up test to only check relevant widened select lines. I checked other tests that other people have written and some of them also check for the relevant lines only, not all lines generated by the python script. So I did the same. Any opinions is this a preferred way?

[fhahn]

LGTM with a few additional changes.

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

20

I think it is more common to pass generic analysis at the end in LLVM. Might be good to adjust.

llvm/test/Transforms/LoopVectorize/vplan-widen-select-instruction.ll

2

Looks like the tests are not auto-generated. Probably best to drop this?

29

the body could probably be a bit further simplified by getting rid of the reduction in the inner loop and instead storing the select result directly in the inner loop (instead of storing sum in the outer loop latch). Same for the other tests.

With the reduced loops, could you add checks ensuring the the select is also in the expected blocks and the operands and result of the select are the values we expect/used as expected?

[a.elovikov]

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

75

@fhahn, are there any plans to start using DivergenceAnalysis instead of SE for the purpose of checking the uniformity?

[Kazhuu]

Simplify the test case and add more checks to make sure select statement operands and result are used correctly.

[Kazhuu]

If the rest of the changes are okay then I need someone to commit this for me. Commit can be made with name Mauri Mustonen and email [e-mail], thanks!

[Kazhuu]

Rebase

[Kazhuu]

Rebase to avoid failing tests.

[fhahn]

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

75

Not that I am aware of at the moment.