[PATCH] D123005: [VPlan] Use region for each loop in native path.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal accepted this revision.
Ayal added a comment.
This revision is now accepted and ready to land.

This looks good to me, ship it!



================
Comment at: llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp:120
 
+  // Get or create a region for the loop containing BB.
+  Loop *CurrentLoop = LI->getLoopFor(BB);
----------------
fhahn wrote:
> Ayal wrote:
> > Deserves a separate getOrCreate[EnclosingLoop]Region()?
> Given that is is just used here I'm not sure if it is worth pulling this out. Could be a lambda if you think early exits would help readability?
Added comment suffices to help readability, thanks!


================
Comment at: llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp:267
          "Unexpected loop preheader");
   VPBasicBlock *PreheaderVPBB = getOrCreateVPBB(PreheaderBB);
   for (auto &I : *PreheaderBB) {
----------------
fhahn wrote:
> Ayal wrote:
> > PreheaderVPBB >> VPlanPreheaderVPBB, to emphasize it's VPlan's outer/entry VPBB, to be returned at the end?
> Done, thanks!
nit: could alternatively use the "The" prefix to denote elements of the topmost/outer loop/region, following `TheLoop`: ThePreheaderBB, ThePreheaderVPBB, ...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123005