[PATCH] D89566: [LV] Epilogue Vectorization with Optimal Control Flow

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added a comment.

Thank you for your reviews @SjoerdMeijer @fhahn .

I totally see the benefit of D92129 <https://reviews.llvm.org/D92129> as you illustrated in D92132 <https://reviews.llvm.org/D92132> and I think it can be used to replace things like `fixupIVUsers()` which would also make it easier to extend epilogue vectorization. As for broader subclassing, it shouldn't prevent us from extending it in a way that uses VPlan. If specialization at the subclass level is not necessary (ie there is code reuse opportunity) then we should still implement it in the superclasses and inherit it in the subclasses. If specialization is not applicable to any other part of the transform then it will happen at the subclass level where it belongs, //whether using VPlan or not//.

Having said that there is a timing aspect. I see your point that if we tried to extend epilogue vectorization now via specialization of parts of the codegen, //that don't use VPlan currently//, then we'd have to change more places in the future. From that perspective a more targeted specialization would help by making it harder to extend epilogue vectorization without having improved VPlan. On the other hand, some improvements to the epilogue vectorization would become contingent on improving VPlan (which is not an issue for me, but others may disagree).

Having a skeleton creator class would require some refactoring of `InnerLoopVectorizer` and `LoopVectorizationPlanner`, so it would warrant having a separate patch for it. I can do it after this one lands.

BTW, I found a trick to make some of the tests target independent and expand the coverage. Please see `llvm/test/Transforms/LoopVectorize/optimal-epilog-vectorization*`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89566/new/

https://reviews.llvm.org/D89566