[PATCH] D111125: [NFC][LoopVectorize] Remove setBestPlan in favour of getBestPlanFor

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added a comment.

In D111125#3044674 <https://reviews.llvm.org/D111125#3044674>, @david-arm wrote:

> In D111125#3043297 <https://reviews.llvm.org/D111125#3043297>, @bmahjour wrote:
>
>> Seems like a reasonable idea to me, but just pointing out that the lifetime of unused plans will be extended with this patch. Could we delete unused plans, eg by calling something like `LVP.deleteVPlansExcept(BestPlan2)` right before `LVP.executePlan(...,)` on line 10435?
>
> Hi @bmahjour, I think that's basically what I was doing in D109432 <https://reviews.llvm.org/D109432>, where I backed up a copy of the plans before deleting them. @sdesmalen suggested I could take a different approach with this patch instead. The problem is that once the plans are deleted I can't get them back again, and the scalable VFs and fixed-width VFs live in different plans. This meant that if I deleted the plans when emitting the main vector loop, I then had to find a way of reinstating the plans so I could then generate the epilogue vector loop.

Yes, but say the plan for the main loop (scalable) is P1 <https://reviews.llvm.org/P1> and the plan for the epilogue (fixed-width) is P2 <https://reviews.llvm.org/P2>. Assuming P1 <https://reviews.llvm.org/P1> `!=` P2 <https://reviews.llvm.org/P2>, once P1 <https://reviews.llvm.org/P1> is executed, we don't need it anymore, so we can delete it right before executing P2 <https://reviews.llvm.org/P2>. Similarly for the path where epilogue is not vectorized, we could delete all other plans before executing the "best plan". Am I misunderstanding something?



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationPlanner.h:299
+  /// Return the best VPlan for the <VF,UF> pair.
+  VPlanPtr& getBestPlanFor(ElementCount VF, unsigned UF);
 
----------------
bmahjour wrote:
> could you make this function `const`?
sorry I meant making the member function constant, but the return type would be good to be const as well (ie `const VPlanPtr &getBestPlanFor(ElementCount VF, unsigned UF) const;`)


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

https://reviews.llvm.org/D111125