[PATCH] D109432: [LoopVectorize] Permit fixed-width epilogue loops for scalable vector bodies

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8200
+      if (!I.get()->hasVF(VF))
+        BackupPlans->push_back(std::unique_ptr<llvm::VPlan>(I.release()));
+    }
----------------
david-arm wrote:
> sdesmalen wrote:
> > I see that you're just building on top of the current mechanism, but I'd rather see `setBestPlan` replaced by `getBestPlanFor`, which returns a pointer to the VPlan that can be passed to `LoopVectorizationPlanner::executePlan`. That way, you don't need to do all this odd trickery with removing vplans and requiring `BackupPlans` to repopulate the set for a second call to `setBestPlan`.
> > 
> > Then the code becomes a bit simpler to follow:
> > 
> >   auto Plan = getBestPlanFor(VF, UF)
> >   LVP.executePlan(Plan, VF, UF, ILV, DT);
> > 
> > I don't know whether the type of `auto Plan` can be `VPlan*` or whether it needs to be an instance of `std::unique_ptr<VPlan>`. It would be convenient if the LoopVectorizationPlanner can keep ownership of all VPlans until the end, where `LoopVectorizationPlanner::executePlan` just invokes the relevant VPlan to execute.
> > 
> Hi @sdesmalen, Im happy to take a look at this. I think in order to keep the VPlans in LoopVectorizationPlanner and return a pointer to the best vplan from `getBestPlanFor` I'd need to change VPlans to use `VPlan*` instead of `std::unique_ptr<VPlan>`. Not sure if @fhahn or @bmahjour have any thoughts on this?
Actually, creating function prototypes like this seems to work:

  PlanPtr& getBestPlanFor(ElementCount VF, unsigned UF);
  void executePlan(VPlanPtr &BestPlan, InnerLoopVectorizer &LB, DominatorTree *DT);



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

https://reviews.llvm.org/D109432