Page MenuHomePhabricator

[SVE] Add support for scalable vectorization of loops with selects and cmps
ClosedPublic

Authored by david-arm on Jan 20 2021, 5:54 AM.

Details

Summary

I have removed an unnecessary assert in LoopVectorizationCostModel::getInstructionCost
that prevented a cost being calculated for select instructions when using
scalable vectors. In addition, I have changed AArch64TTIImpl::getCmpSelInstrCost
to only do special cost calculations for fixed width vectors and fall
back to the base version for scalable vectors.

I have added a simple cost model test for cmps and selects:

test/Analysis/CostModel/sve-cmpsel.ll

and some simple tests that show we vectorize loops with cmp and select:

test/Transforms/LoopVectorize/AArch64/sve-basic-vec.ll

Diff Detail

Event Timeline

david-arm created this revision.Jan 20 2021, 5:54 AM
david-arm requested review of this revision.Jan 20 2021, 5:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 20 2021, 5:54 AM
david-arm added inline comments.Jan 20 2021, 5:56 AM
llvm/test/Analysis/CostModel/sve-cmpsel.ll
9

Here I've deliberately tried to avoid an explosion of test cases, so for the legal and illegal variants I used different element types.

dmgreen added inline comments.
llvm/test/Analysis/CostModel/sve-cmpsel.ll
9

Cost model checks don't really need valid inputs, they can just use undef. It makes adding a lot of them much simpler. See something like llvm/test/Analysis/CostModel/ARM/reduce-smax.ll or llvm/test/Analysis/CostModel/X86/arith-fma.ll. Even though they are adding many (sub-) architectures, they are still managable.

david-arm added inline comments.Jan 20 2021, 8:53 AM
llvm/test/Transforms/LoopVectorize/AArch64/sve-basic-vec.ll
12

Apologies, just realised these names are terrible! I'll fix them in another patch. I also wonder if autogenerating the CHECK lines really makes sense here.

david-arm updated this revision to Diff 318148.Jan 21 2021, 4:05 AM
  • Added more cost model test cases and optimised existing ones.
  • Renamed vectorisation tests to something more useful. :) I also reduced the number of CHECK lines as it looked too messy and fragile.
david-arm marked an inline comment as done.Jan 21 2021, 4:07 AM
david-arm added inline comments.
llvm/test/Analysis/CostModel/sve-cmpsel.ll
9

Thanks for the suggestion @dmgreen!

dmgreen accepted this revision.Jan 21 2021, 6:40 AM

The code changes look simple enough, LGTM.

Up to you if you want to try and reduce the test boilerplate too.

llvm/test/Analysis/CostModel/sve-cmpsel.ll
9

I would combine all these into less functions to reduce all this boilerplate. Maybe one function for the cmp's and one for the sel's? Or maybe split out more into the legal/illegal types, like you have commented. They don't need to actually use the output of the instruction.

It makes adding lots of test much easier to check, but up to you what you think.

This revision is now accepted and ready to land.Jan 21 2021, 6:40 AM
This revision was automatically updated to reflect the committed changes.
david-arm marked an inline comment as done.