This is an archive of the discontinued LLVM Phabricator instance.

[LV] Enable scalable outer loop vectorization
Needs ReviewPublic

Authored by nikolaypanchenko on Apr 28 2023, 1:05 PM.

Details

Summary

The changeset allows VLA outer loop vectorization as well as

  • fixes incorrect handling of a loopnest during HCFG VPlan construction
  • adds vec remark when outer loop was successfully vectorized

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptApr 28 2023, 1:05 PM
nikolaypanchenko requested review of this revision.Apr 28 2023, 1:05 PM
fhahn added inline comments.May 3 2023, 12:09 PM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
2250

Could this improvement be split off as a separate patch? It also looks like LVL & LVP are unused?

llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
123

Can this be split off as well with a separate test?

llvm/test/Transforms/LoopVectorize/RISCV/outer_loop_scalable.ll
98

Could you update the tests to use names for blocks and values? This would make them much easier to read.

llvm/test/Transforms/LoopVectorize/X86/outer_loop_no_scalable.ll
1

Does this test need to pass an X86 triple?

llvm/test/Transforms/LoopVectorize/outer_loop_test3.ll
3

Is this test for the opt remarks? In that case, it would be good to include it in the name (e.g. outer-loop-remarks.ll)?

iamlouk added a subscriber: iamlouk.Aug 8 2023, 1:07 AM

@nikolaypanchenko, thank you for this patch! Sorry to bother you, but will you update the revision (to support scalable vectorization factors)? I have written a patch myself with the same intention (although it works slightly differently, I also modified determineVPlanVF), not knowing that this revision already exists, so if you do not have the resources for this at the moment, I could present an alternative one, but only if you are ok with that of course.

nikolaypanchenko marked an inline comment as done.Aug 8 2023, 12:52 PM

@nikolaypanchenko, thank you for this patch! Sorry to bother you, but will you update the revision (to support scalable vectorization factors)? I have written a patch myself with the same intention (although it works slightly differently, I also modified determineVPlanVF), not knowing that this revision already exists, so if you do not have the resources for this at the moment, I could present an alternative one, but only if you are ok with that of course.

@iamlouk this patch addressed 3 different problems: outerloop vectorization for scalable vectors; vec-report for outerloop and stability fix for outerloop; As @fhahn suggested, I split last 2 into their own review: D150696 and D150700.
I assume your approach to enable outerloop vectorization for scalable vectors is more generic than mine adhoc approach here, so can you please post your patch instead ?

iamlouk this patch addressed 3 different problems: outerloop vectorization for scalable vectors; vec-report for outerloop and stability fix for outerloop; As fhahn suggested, I split last 2 into their own review: D150696 and D150700.
I assume your approach to enable outerloop vectorization for scalable vectors is more generic than mine adhoc approach here, so can you please post your patch instead ?

Thank you! Here is the review: D157484 , I will happily rebase it in order to add a call to reportVectorization once that patch of yours is commited.