[VPlan] Move initial skeleton construction to createInitialVPlan. (NFC)

[fhahn]

Summary

This patch moves creating the middle VPBBs and an initial empty
vector loop region for the top-level loop to createInitialVPlan.

This consolidates code to create the initial VPlan skeleton and enables
adding other bits outside the main region during initial VPlan
construction. In particular, D150398 will add the exit check & branch to
the middle block.

[fhahn]

Ping :)

[Ayal]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

8773

Moving the creation of top region and middle block into createInitialVPlan looks good, and follows the description above, except for the last sentence. Documentation of createInitialVPlan() in VPlan.h needs updating to keep in sync.
The initial VPlan conceptually consists of pre-header, top-region, and exit, all representing existing IR "in-place". A vector-pre-header is also part if the initial VPlan, currently.

The last sentence above: filling the top-region with header, latch, followed by VPBB's in-between, should take place as independent steps (VPlan-to-VPlan transforms), potentially after caching a cloned copy of the original "in-place" top-region to be used for fallback and/or remainder loops. Possibly as follow-up patches.

8782–8783

Comment line dropped accidentally? BTW, not sure I follow why splitting the range is avoided here nor how taking a uniform conservative decision across all VF's in Range relates to UF being unknown(?).

8900

nit: independent cleanup?

llvm/lib/Transforms/Vectorize/VPlan.cpp

717

nit: worth a comment that the top region is initially constructed empty here, as a placeholder, to be filled later with VPBB's for subsequent processing.

llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp

107

"it will be" >> "the vector-pre-header VPBB was"?

Avoid setting both predecessors of (any) header here, i.e., early-return if BB is a header block, to avoid setting edges to predecessor only to be discarded later?
Likewise, avoid setting successors of latch blocks below, only to be discarded later.
(Possibly as independent preparatory patch(es); make sure important orders are maintained.)

322

nit: can assign VPRegionBlock *TheRegion = Plan.getVectorLoopRegion(); and reuse it below to set

  auto *VectorPreheaderVPBB = cast<VPBasicBlock>(TheRegion->getSinglePredecessor());
  BB2VPBB[ThePreheaderBB] = VectorPreheaderVPBB;
  BB2VPBB[LoopExitBB] = cast<VPBasicBlock>(TheRegion->getSingleSuccessor());
}

Note that ThePreheaderBB is associated with both Plan.getPreheader() and Plan.getEntry(); here we're interested in the latter, which stands for Vector-Preheader.

322

349–350

354–355

drop suffix: PH->H is no longer linked, neither here nor elsewhere.

"Loop H" >> "the header block of the top region."

412–413

412–413

Could the disconnects be avoided, and top region early continue?

413

[fhahn]

Rebased on top of D159136 and address comments, thanks!

[fhahn]

Rebase after updates to D159136.

[Ayal]

llvm/lib/Transforms/Vectorize/VPlan.cpp

719

llvm/lib/Transforms/Vectorize/VPlan.h

2566

llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp

323

// These were created directly rather than via getOrCreateVPBB(), revisit them now to update BB2VPBB and Loop2region.

329

nit:
auto *VectorPreheaderVPBB = cast<VPBasicBlock>(TheRegion->getSinglePredecessor());
would be more consistent with how middle VPBB is retrieved.

330

Worth noting that ThePreheaderBB conceptually corresponds to both Plan.getPreheader() (which wraps the original preheader BB) and Plan.getEntry() (which represents the new vector preheader); here we're interested in setting BB2VPBB to the latter.

349–350

Is this redundant - createInitialVPlan() already sets the name of VectorPreheaderVPBB to "vector.ph" when creating it?

357

Is setting the parent of HeaderVPBB to TheRegion redundant - getOrCreateVPBB() should have done so already?

358

Should this setEntry be done along with all others?

375–376

nit: may be simpler to handle the non-header case first, then potentially else-if the header-of-non-TheRegion case.

394–395

nit: continue if TheRegion == Region?

394–395

If Exits of regions are set here when identifying latches during successor setting, should Entries be set above when identifying headers during predecessor setting?

[fhahn]

Rebase after update to D159136

[fhahn]

Rebase and address comments, thanks!

ping :)

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

8773

Moving the creation of top region and middle block into createInitialVPlan looks good, and follows the description above, except for the last sentence. Documentation of createInitialVPlan() in VPlan.h needs updating to keep in sync.

The initial VPlan conceptually consists of pre-header, top-region, and exit, all representing existing IR "in-place". A vector-pre-header is also part if the initial VPlan, currently.

Updated the comment in VPlan.h to be in sync with the statement here (except last sentence).

The last sentence above: filling the top-region with header, latch, followed by VPBB's in-between, should take place as independent steps (VPlan-to-VPlan transforms), potentially after caching a cloned copy of the original "in-place" top-region to be used for fallback and/or remainder loops. Possibly as follow-up patches.

Creating the header & latch VPBBs is mostly done up-front at the moment, to simplify the code for recipe creation we have now. Going forward, it should probably be possible to eventually just use the blocks created during initial VPlan construction (like in the native path currently) and what's left to do is adding the canonical induction recipes separately.

8782–8783

Ah yes, accidentally remove. Added the comment back, thanks!

8900

Split off to b1115f8ccefb380824a9d997622cc84fc0d84a89

llvm/lib/Transforms/Vectorize/VPlan.cpp

717

Added a comment, thanks!

719

Updated, thanks!

llvm/lib/Transforms/Vectorize/VPlan.h

2566

Fixed, thanks!

llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp

107

I had look at this and it turned out a bit more complicated, but overall simpler and easier to update/change in this patch: D159136

322

Adjusted, thanks!

323

Added, thanks!

329

Updated, thanks!

330

added, thanks!

349–350

Adjusted naming, thanks!

349–350

Indeed, removed, thanks!

354–355

Adjusted, thanks!

357

Indeed, removed, thanks!

358

At the other point we would need extra checks for the block being a header.

375–376

the non-header case should be handled first now

394–395

Entries are set at a different point now.

412–413

This code is now gone, based on D159136

412–413

Adjusted, thanks!

413

Adjusted, thanks!

[Ayal]

In summary: "This patch moves creating the preheader and middle VPBBs as well as the
initial empty vector loop region to createInitialVPlan" - only middle VPBB and region are moved, preheader is already there.

llvm/lib/Transforms/Vectorize/VPlan.cpp

712

This should be auto Entry = new VPBasicBlock("entry") plus updating VPlan's "entry" to refer to this VPBB rather than VecPreHeader. Can be done in a separate patch.

llvm/lib/Transforms/Vectorize/VPlan.h

2564–2568

The original scalar pre-header no longer acts as a pre-header in VPlan's skeleton and best be renamed "entry". Can be done as part of rewording VPlan's "Entry" and "VectorPreHeader" in a separate patch.

llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp

182

If LoopOfBB == TheLoop then RegionExists because it was constructed in buildInitialVPlan, but the header VPBB of TheLoop is not - it gets constructed by calling getOrCreateVPBB(). I.e., that BB is exceptional in being a header (not in BB2VPBB) of an existing region (in Loop2Region).

185

Better to assert !isHeaderBB(BB, LoopOfBB) || LoopOfBB == TheLoop here?

186

(otherwise VPBB's parent gets set to a new region upon constructing the latter below.)

193

Better to assert isHeaderBB(BB, LoopOfBB) here?

194–195

324

324

Note that header/entry and latch/exiting VPBB's of top-level region have yet to be created.

357

OTOH, perhaps its clearer to create HeaderVPBB next to stage 0 above:

BasicBlock *HeaderBB = TheLoop->getHeader();
VPBasicBlock *HeaderVPBB = new VPBasicBlock(HeaderBB->getName());
BB2VPBB[HeaderBB] = HeaderVPBB;
HeaderVPBB->setParent(TheRegion);

thereby also slightly simplifying getOrCreateVPBB?

369–371

371–373

// Except for the top region, whose predecessor was set when creating VPlan's skeleton.

403–404

// Except for the top region, whose successor was set when creating VPlan's skeleton.

[fhahn]

Address latest comments, thanks!

In D158333#4654821, @Ayal wrote:

In summary: "This patch moves creating the preheader and middle VPBBs as well as the
initial empty vector loop region to createInitialVPlan" - only middle VPBB and region are moved, preheader is already there.

Adjusted, thanks!

[Ayal]

In D158333#4654836, @fhahn wrote:

Address latest comments, thanks!

In D158333#4654821, @Ayal wrote:

In summary: "This patch moves creating the preheader and middle VPBBs as well as the
initial empty vector loop region to createInitialVPlan" - only middle VPBB and region are moved, preheader is already there.

Adjusted, thanks!

There are still some pending comments, right? In particular regarding simplifying getOrCreateVPBB() wrt the exceptional header VPBB of TheLoop.

[fhahn]

Added early exit if BB is header of TheLoop to getOrCreateVPBB, also submit pending responses.

llvm/lib/Transforms/Vectorize/VPlan.cpp

712

Will do separately, thanks!

llvm/lib/Transforms/Vectorize/VPlan.h

2564–2568

Clarified, thanks!

llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp

182

Handled the case hat BB is the header of TheLoop in the early exit above, as setEntry in the caller will take care of setting the region. This simplifies the asserts below.

185

Added assert, thanks!

193

added assert, thanks!

194–195

Simplified, thanks!

324

Added the note, thanks!

369–371

Adjusted, thanks!

371–373

Clarified, thanks!

403–404

Clarified, thanks!

[Ayal]

Still wonder if the exceptional case of top region's header VPBB could be dealt with better, if not entirely outside getOrCreateVPBB() then perhaps entirely inside it?

llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp

178

Better have getOrCreateVPBB() fully handle TheLoop's header (see below), and retain this early-exit only if !LoopOfBB? (Or explain its partial treatment here - region created (and registered in Loop2Region) before the call but hooking up VPBB as its entry/parent and fixing its name is left for the caller to fix after the call.)

181–182

This assert to be removed now?

182

An alternative to early exiting, leaving setEntry to the caller, is to provide full treatment for top loop header BB; would something as follows work better, also taking care of updating Loop2Region?

  auto *RegionOfVPBB = Loop2Region.lookup(LoopOfBB);
  if (!isHeaderBB(BB, LoopOfBB)) {
    assert(RegionOfVPBB && "Region should have been created by visiting header earlier");
    VPBB->setParent(RegionOfVPBB);
    return VPBB;
  }

  // Handle a header - take care of its Region.
  assert(!RegionOfVPBB && "...");
  if (LoopOfBB == TheLoop) {
    RegionOfVPBB = Plan.getVectorLoopRegion();
  } else {
    RegionOfVPBB =
      new VPRegionBlock(BB->getName().str(), false /*isReplicator*/);
    RegionOfVPBB->setParent(Loop2Region[LoopOfBB->getParentLoop()]);
  }
  RegionOfVPBB->setEntry(VPBB);
  Loop2Region[LoopOfBB] = RegionOfVPBB;
  return VPBB;
}

327

fold this into its natural place in getOrCreateVPBB()?

354–355

nit: cleaner to provide the name as an (optional) parameter to getOrCreateVPBB()?

355–356

fold this into its natural place in getOrCreateVPBB()?

[fhahn]

llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp

178

Updated, thanks! The caller now doesn't add the region to Loop2Region, all headers are handled here now.

181–182

removed, thanks!

182

Updated, thanks!

327

Done, thanks!

354–355

Done, thanks!

355–356

Done, thanks!

[fhahn]

In D158333#4656541, @Ayal wrote:

Still wonder if the exceptional case of top region's header VPBB could be dealt with better, if not entirely outside getOrCreateVPBB() then perhaps entirely inside it?

Adjusted as suggested, thanks!

[Ayal]

Thanks for accommodating! A couple of final cleanups and nits.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

8783

nit: worth adding a note to revisit this comment, unless fully agreed upon?

llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp

171

Alternatively, can check if isHeaderBB(BB, TheLoop) and set its name to "vector.body". This method already handles this case, and taking care of the name here would simplify and unify the caller(s) below.

Would be good if these debug messages were covered by some test.

189

... should be filled in with some message, e.g., "First visit of a header basic block expects to register its region"

195

?

324

325

355–356

If getOrCreateVPBB() takes care of all blocks equally, creating VPBB for header block of top region can be done as part of the RPO scan below, along with all other VPBB's?

[fhahn]

Address comments, thanks!

[fhahn]

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

8783

Added, thanks!

llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp

171

Adjusted, thanks!

189

Forgot about this one, added, thanks!

195

Adjusted, thanks!

324

Adjusted, thanks!

325

Adjusted, thanks!

355–356

Removed, thanks!

[Ayal]

Thanks, Ship it!

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

LoopVectorizationPlanner::tryToBuildVPlanWithVPRecipes(VFRange &Range) {
   // Build initial VPlan: Scan the body of the loop in a topological order to
   // visit each basic block after having visited its predecessor basic blocks.
   // ---------------------------------------------------------------------------
 
   // Create initial VPlan skeleton, having a basic block for the pre-header
   // which contains SCEV expansions that need to happen before the CFG is
   // modified; a basic block for the vector pre-header, followed by a region for
   // the vector loop, followed by the middle basic block. The skeleton vector
   // loop region contains a header and latch basic blocks.
   VPlanPtr Plan = VPlan::createInitialVPlan(
       createTripCountSCEV(Legal->getWidestInductionType(), PSE, OrigLoop),
       *PSE.getSE());
   VPBasicBlock *HeaderVPBB = new VPBasicBlock("vector.body");
   VPBasicBlock *LatchVPBB = new VPBasicBlock("vector.latch");
   VPBlockUtils::insertBlockAfter(LatchVPBB, HeaderVPBB);
-  auto *TopRegion = new VPRegionBlock(HeaderVPBB, LatchVPBB, "vector loop");
-  VPBlockUtils::insertBlockAfter(TopRegion, Plan->getEntry());
-  VPBasicBlock *MiddleVPBB = new VPBasicBlock("middle.block");
-  VPBlockUtils::insertBlockAfter(MiddleVPBB, TopRegion);
+  Plan->getVectorLoopRegion()->setEntry(HeaderVPBB);
+  Plan->getVectorLoopRegion()->setExiting(LatchVPBB);
 
   // Don't use getDecisionAndClampRange here, because we don't know the UF
   // so this function is better to be conservative, rather than to split
   // it up into different VPlans.
   bool IVUpdateMayOverflow = false;
   for (ElementCount VF : Range)
     IVUpdateMayOverflow |= !isIndvarOverflowCheckKnownFalse(&CM, VF);
 
   DebugLoc DL = getDebugLocFromInstOrOperands(Legal->getPrimaryInduction());
   TailFoldingStyle Style = CM.getTailFoldingStyle(IVUpdateMayOverflow);
LoopVectorizationPlanner::tryToBuildVPlanWithVPRecipes(VFRange &Range) {
   RecipeBuilder.fixHeaderPhis();
 
   // ---------------------------------------------------------------------------
   // Transform initial VPlan: Apply previously taken decisions, in order, to
   // bring the VPlan to its final state.
   // ---------------------------------------------------------------------------
 
   // Adjust the recipes for any inloop reductions.
   adjustRecipesForReductions(LatchVPBB, Plan, RecipeBuilder, Range.Start);
 
   // Interleave memory: for each Interleave Group we marked earlier as relevant
   // for this VPlan, replace the Recipes widening its memory instructions with a
   // single VPInterleaveRecipe at its insertion point.
   for (const auto *IG : InterleaveGroups) {
     auto *Recipe = cast<VPWidenMemoryInstructionRecipe>(
         RecipeBuilder.getRecipe(IG->getInsertPos()));
     SmallVector<VPValue *, 4> StoredValues;

llvm/lib/Transforms/Vectorize/VPlan.h

VPlan(VPBasicBlock *Preheader, VPBasicBlock *Entry)
     Preheader->setPlan(this);
     assert(Preheader->getNumSuccessors() == 0 &&
            Preheader->getNumPredecessors() == 0 &&
            "preheader must be disconnected");
   }
 
   ~VPlan();
 
-  /// Create an initial VPlan with preheader and entry blocks. Creates a
-  /// VPExpandSCEVRecipe for \p TripCount and uses it as plan's trip count.
+  /// Create initial VPlan skeleton, having an "entry" VPBasicBlock (wrapping
+  /// original scalar pre-header) which contains SCEV expansions that need to
+  /// happen before the CFG is modified; a VPBasicBlock for the vector
+  /// pre-header, followed by a region for the vector loop, followed by the
+  /// middle VPBasicBlock.
   static VPlanPtr createInitialVPlan(const SCEV *TripCount,
                                      ScalarEvolution &PSE);
 
   /// Prepare the plan for execution, setting up the required live-in values.
   void prepareToExecute(Value *TripCount, Value *VectorTripCount,
                         Value *CanonicalIVStartValue, VPTransformState &State,
                         bool IsEpilogueVectorization);
 

llvm/lib/Transforms/Vectorize/VPlan.cpp

VPlan::~VPlan() {
   }
   for (VPValue *VPV : VPLiveInsToFree)
     delete VPV;
   if (BackedgeTakenCount)
     delete BackedgeTakenCount;
 }
 
 VPlanPtr VPlan::createInitialVPlan(const SCEV *TripCount, ScalarEvolution &SE) {
   VPBasicBlock *Preheader = new VPBasicBlock("ph");
   VPBasicBlock *VecPreheader = new VPBasicBlock("vector.ph");
   auto Plan = std::make_unique<VPlan>(Preheader, VecPreheader);
   Plan->TripCount =
       vputils::getOrCreateVPValueForSCEVExpr(*Plan, TripCount, SE);
+  // Create empty VPRegionBlock, to be filled during processing later.
+  auto *TopRegion = new VPRegionBlock("vector loop", false /*isReplicator*/);
+  VPBlockUtils::insertBlockAfter(TopRegion, VecPreheader);
+  VPBasicBlock *MiddleVPBB = new VPBasicBlock("middle.block");
+  VPBlockUtils::insertBlockAfter(MiddleVPBB, TopRegion);
   return Plan;
 }
 
 void VPlan::prepareToExecute(Value *TripCountV, Value *VectorTripCountV,
                              Value *CanonicalIVStartValue,
                              VPTransformState &State,
                              bool IsEpilogueVectorization) {
   // Check if the backedge taken count is needed, and if so build it.

llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp

private:
 
   /// Maps loops in the original IR to their corresponding region.
   DenseMap<Loop *, VPRegionBlock *> Loop2Region;
 
   // Utility functions.
   void setVPBBPredsFromBB(VPBasicBlock *VPBB, BasicBlock *BB);
   void setRegionPredsFromBB(VPRegionBlock *VPBB, BasicBlock *BB);
   void fixPhiNodes();
-  VPBasicBlock *getOrCreateVPBB(BasicBlock *BB);
+  VPBasicBlock *getOrCreateVPBB(BasicBlock *BB, StringRef Name = "");
 #ifndef NDEBUG
   bool isExternalDef(Value *Val);
 #endif
   VPValue *getOrCreateVPOperand(Value *IRVal);
   void createVPInstructionsForVPBB(VPBasicBlock *VPBB, BasicBlock *BB);
 
 public:
   PlainCFGBuilder(Loop *Lp, LoopInfo *LI, VPlan &P)
assert(VPBB == cast<VPBasicBlock>(PredRegion->getSingleSuccessor()) &&
            "successor must already be set for PredRegion; it must have VPBB "
            "as single successor");
     VPBB->setPredecessors({PredRegion});
     return;
   }
   // Collect VPBB predecessors.
   SmallVector<VPBlockBase *, 2> VPBBPreds;
   for (BasicBlock *Pred : predecessors(BB))
     VPBBPreds.push_back(getOrCreateVPBB(Pred));
   VPBB->setPredecessors(VPBBPreds);
 }
 
 static bool isHeaderBB(BasicBlock *BB, Loop *L) {
   return L && BB == L->getHeader();
 }
 
 void PlainCFGBuilder::setRegionPredsFromBB(VPRegionBlock *Region,

 static bool isHeaderVPBB(VPBasicBlock *VPBB) {
   return VPBB->getParent() && VPBB->getParent()->getEntry() == VPBB;
 }
 
 // Create a new empty VPBasicBlock for an incoming BasicBlock in the region
 // corresponding to the containing loop  or retrieve an existing one if it was
 // already created. If no region exists yet for the loop containing \p BB, a new
 // one is created.
-VPBasicBlock *PlainCFGBuilder::getOrCreateVPBB(BasicBlock *BB) {
+VPBasicBlock *PlainCFGBuilder::getOrCreateVPBB(BasicBlock *BB, StringRef Name) {
   if (auto *VPBB = BB2VPBB.lookup(BB)) {
     // Retrieve existing VPBB.
     return VPBB;
   }
 
   // Create new VPBB.
-  LLVM_DEBUG(dbgs() << "Creating VPBasicBlock for " << BB->getName() << "\n");
-  VPBasicBlock *VPBB = new VPBasicBlock(BB->getName());
+  if (Name.size() == 0)
+    Name = BB->getName();
+  LLVM_DEBUG(dbgs() << "Creating VPBasicBlock for " << Name << "\n");
+  VPBasicBlock *VPBB = new VPBasicBlock(Name);
   BB2VPBB[BB] = VPBB;
 
   // Get or create a region for the loop containing BB.
   Loop *LoopOfBB = LI->getLoopFor(BB);
   if (!LoopOfBB)
     return VPBB;
 
-  VPRegionBlock *RegionOfBB = Loop2Region.lookup(LoopOfBB);
-  assert((RegionOfBB != nullptr) ^ isHeaderBB(BB, LoopOfBB) &&
-         "region must exist or BB must be a loop header");
-  if (RegionOfBB) {
-    VPBB->setParent(RegionOfBB);
+  auto *RegionOfVPBB = Loop2Region.lookup(LoopOfBB);
+  if (!isHeaderBB(BB, LoopOfBB)) {
+    assert(RegionOfVPBB &&
+           "Region should have been created by visiting header earlier");
+    VPBB->setParent(RegionOfVPBB);
+    return VPBB;
+  }
+
+  assert(!RegionOfVPBB && "...");
+  // Handle a header - take care of its Region.
+  if (LoopOfBB == TheLoop) {
+    RegionOfVPBB = Plan.getVectorLoopRegion();
   } else {
-    // If BB's loop is nested inside another loop within VPlan's scope, the
-    // header of that enclosing loop was already visited and its region
-    // constructed and recorded in Loop2Region. That region is now set as the
-    // parent of VPBB's region. Otherwise it is set to null.
-    auto *RegionOfVPBB = new VPRegionBlock(
-        LoopOfBB->getHeader()->getName().str(), false /*isReplicator*/);
+    RegionOfVPBB =
+        new VPRegionBlock(BB->getName().str(), false /*isReplicator*/);
     RegionOfVPBB->setParent(Loop2Region[LoopOfBB->getParentLoop()]);
-    RegionOfVPBB->setEntry(VPBB);
-    Loop2Region[LoopOfBB] = RegionOfVPBB;
-  }
+  }
   return VPBB;
 }
 
 #ifndef NDEBUG
 // Return true if \p Val is considered an external definition. An external
 // definition is either:
 // 1. A Value that is not an Instruction. This will be refined in the future.
 // 2. An Instruction that is outside of the CFG snippet represented in VPlan,
for (Instruction &InstRef : *BB) {
     }
 
     IRDef2VPValue[Inst] = NewVPV;
   }
 }
 
 // Main interface to build the plain CFG.
 void PlainCFGBuilder::buildPlainCFG() {
+  // 0. Reuse the top-level region, vector-preheader and exit VPBBs from the
+  // skeleton. These were created directly rather than via getOrCreateVPBB(),
+  // revisit them now to update BB2VPBB and Loop2region. Note that header/entry
+  // and latch/exiting VPBB's of top-level region have yet to be created
+  VPRegionBlock *TheRegion = Plan.getVectorLoopRegion();
+  BasicBlock *ThePreheaderBB = TheLoop->getLoopPreheader();
+  assert((ThePreheaderBB->getTerminator()->getNumSuccessors() == 1) &&
+         "Unexpected loop preheader");
+  auto *VectorPreheaderVPBB =
+      cast<VPBasicBlock>(TheRegion->getSinglePredecessor());
+  // ThePreheaderBB conceptually corresponds to both Plan.getPreheader() (which
+  // wraps the original preheader BB) and Plan.getEntry() (which represents the
+  // new vector preheader); here we're interested in setting BB2VPBB to the
+  // latter.
+  BB2VPBB[ThePreheaderBB] = VectorPreheaderVPBB;
+  BasicBlock *LoopExitBB = TheLoop->getUniqueExitBlock();
+  assert(LoopExitBB && "Loops with multiple exits are not supported.");
+  BB2VPBB[LoopExitBB] = cast<VPBasicBlock>(TheRegion->getSingleSuccessor());
+
   // 1. Scan the body of the loop in a topological order to visit each basic
   // block after having visited its predecessor basic blocks. Create a VPBB for
   // each BB and link it to its successor and predecessor VPBBs. Note that
   // predecessors must be set in the same order as they are in the incomming IR.
   // Otherwise, there might be problems with existing phi nodes and algorithm
   // based on predecessors traversal.
 
   // Loop PH needs to be explicitly visited since it's not taken into account by
   // LoopBlocksDFS.
-  BasicBlock *ThePreheaderBB = TheLoop->getLoopPreheader();
-  assert((ThePreheaderBB->getTerminator()->getNumSuccessors() == 1) &&
-         "Unexpected loop preheader");
-  // buildPlainCFG needs to be called after createInitialVPlan, which creates
-  // the initial skeleton (including the preheader VPBB). buildPlainCFG builds
-  // the CFG for the loop nest and hooks it up to the initial skeleton.
-  VPBasicBlock *ThePreheaderVPBB = Plan.getEntry();
-  BB2VPBB[ThePreheaderBB] = ThePreheaderVPBB;
-  ThePreheaderVPBB->setName("vector.ph");
   for (auto &I : *ThePreheaderBB) {
     if (I.getType()->isVoidTy())
       continue;
     IRDef2VPValue[&I] = Plan.getVPValueOrAddLiveIn(&I);
   }
-  // Create region (and header block) for the outer loop, so that we can link
-  // PH->Region.
-  VPBlockBase *HeaderVPBB = getOrCreateVPBB(TheLoop->getHeader());
-  HeaderVPBB->setName("vector.body");
-  ThePreheaderVPBB->setOneSuccessor(HeaderVPBB->getParent());
+  // Create empty VPBB for header block of the top region and set its name.
+  getOrCreateVPBB(TheLoop->getHeader(), "vector.body");
 
   LoopBlocksRPO RPO(TheLoop);
   RPO.perform(LI);
 
   for (BasicBlock *BB : RPO) {
     // Create or retrieve the VPBasicBlock for this BB and create its
     // VPInstructions.
     VPBasicBlock *VPBB = getOrCreateVPBB(BB);
     VPRegionBlock *Region = VPBB->getParent();
     createVPInstructionsForVPBB(VPBB, BB);
     Loop *LoopForBB = LI->getLoopFor(BB);
     // Set VPBB predecessors in the same order as they are in the incoming BB.
-    if (!isHeaderBB(BB, LoopForBB))
+    if (!isHeaderBB(BB, LoopForBB)) {
       setVPBBPredsFromBB(VPBB, BB);
-    else {
-      // BB is a loop header, set the predecessor for the region.
+    } else {
+      // BB is a loop header, set the predecessor for the region, except for the
+      // top region, whose predecessor was set when creating VPlan's skeleton.
       assert(isHeaderVPBB(VPBB) && "isHeaderBB and isHeaderVPBB disagree");
+      if (TheRegion != Region)
         setRegionPredsFromBB(Region, BB);
     }
 
     // Set VPBB successors. We create empty VPBBs for successors if they don't
     // exist already. Recipes will be created when the successor is visited
     // during the RPO traversal.
     auto *BI = cast<BranchInst>(BB->getTerminator());
     unsigned NumSuccs = succ_size(BB);
     if (NumSuccs == 1) {
       auto *Successor = getOrCreateVPBB(BB->getSingleSuccessor());
       VPBB->setOneSuccessor(isHeaderVPBB(Successor)
                                 ? Successor->getParent()
                                 : static_cast<VPBlockBase *>(Successor));
       continue;
     }
     assert(BI->isConditional() && NumSuccs == 2 && BI->isConditional() &&
            "block must have conditional branch with 2 successors");
     // Look up the branch condition to get the corresponding VPValue
     // representing the condition bit in VPlan (which may be in another VPBB).
     assert(IRDef2VPValue.contains(BI->getCondition()) &&
            "Missing condition bit in IRDef2VPValue!");
     VPBasicBlock *Successor0 = getOrCreateVPBB(BI->getSuccessor(0));
     VPBasicBlock *Successor1 = getOrCreateVPBB(BI->getSuccessor(1));
     if (!LoopForBB || BB != LoopForBB->getLoopLatch()) {
       VPBB->setTwoSuccessors(Successor0, Successor1);
       continue;
     }
     // For a latch we need to set the successor of the region rather than that
-    // of VPBB and it should be set to the exit, i.e., non-header successor.
-    Region->setOneSuccessor(isHeaderVPBB(Successor0) ? Successor1 : Successor0);
+    // of VPBB and it should be set to the exit, i.e., non-header successor,
+    // except for the top region, whose successor was set when creating VPlan's
+    // skeleton.
+    if (TheRegion != Region)
+      Region->setOneSuccessor(isHeaderVPBB(Successor0) ? Successor1
+                                                       : Successor0);
     Region->setExiting(VPBB);
   }
 
-  // 2. Process outermost loop exit. We created an empty VPBB for the loop
-  // single exit BB during the RPO traversal of the loop body but Instructions
-  // weren't visited because it's not part of the loop.
-  BasicBlock *LoopExitBB = TheLoop->getUniqueExitBlock();
-  assert(LoopExitBB && "Loops with multiple exits are not supported.");
-  VPBasicBlock *LoopExitVPBB = BB2VPBB[LoopExitBB];
-  // Loop exit was already set as successor of the loop exiting BB.
-  // We only set its predecessor VPBB now.
-  setVPBBPredsFromBB(LoopExitVPBB, LoopExitBB);
-
-  // 3. The whole CFG has been built at this point so all the input Values must
+  // 2. The whole CFG has been built at this point so all the input Values must
   // have a VPlan couterpart. Fix VPlan phi nodes by adding their corresponding
   // VPlan operands.
   fixPhiNodes();
 }
 
 void VPlanHCFGBuilder::buildPlainCFG() {
   PlainCFGBuilder PCFGBuilder(TheLoop, LI, Plan);
   PCFGBuilder.buildPlainCFG();

llvm/test/Transforms/LoopVectorize/vplan-printing-outer-loop.ll

 ; REQUIRES: asserts
 
 ; RUN: opt -passes=loop-vectorize -pass-remarks=loop-vectorize -enable-vplan-native-path -debug -disable-output %s 2>&1 | FileCheck %s
 
 @arr2 = external global [8 x i64], align 16
 @arr = external global [8 x [8 x i64]], align 16
 
 define void @foo(i64 %n) {
 ; CHECK:      VPlan 'HCFGBuilder: Plain CFG
 ; CHECK-NEXT: {
 ; CHECK-NEXT: ir<8> = original trip-count
 ; CHECK-EMPTY:
 ; CHECK-NEXT: vector.ph:
-; CHECK-NEXT: Successor(s): outer.header
+; CHECK-NEXT: Successor(s): vector loop
 ; CHECK-EMPTY:
-; CHECK-NEXT: <x1> outer.header: {
+; CHECK-NEXT: <x1> vector loop: {
 ; CHECK-NEXT:   vector.body:
 ; CHECK-NEXT:     WIDEN-PHI ir<%outer.iv> = phi ir<0>, ir<%outer.iv.next>
 ; CHECK-NEXT:     EMIT ir<%gep.1> = getelementptr ir<@arr2>, ir<0>, ir<%outer.iv>
 ; CHECK-NEXT:     EMIT store ir<%outer.iv>, ir<%gep.1>
 ; CHECK-NEXT:     EMIT ir<%add> = add ir<%outer.iv>, ir<%n>
 ; CHECK-NEXT:   Successor(s): inner
 ; CHECK-EMPTY:
 ; CHECK-NEXT:   <x1> inner: {

 ; CHECK-NEXT:  Successor(s): outer.latch
 ; CHECK-EMPTY:
 ; CHECK-NEXT:   outer.latch:
 ; CHECK-NEXT:     EMIT ir<%outer.iv.next> = add ir<%outer.iv>, ir<1>
 ; CHECK-NEXT:     EMIT ir<%outer.ec> = icmp ir<%outer.iv.next>, ir<8>
 ; CHECK-NEXT:     EMIT branch-on-cond ir<%outer.ec>
 ; CHECK-NEXT:   No successors
 ; CHECK-NEXT:  }
-; CHECK-NEXT: Successor(s): exit
+; CHECK-NEXT: Successor(s): middle.block
 ; CHECK-EMPTY:
-; CHECK-NEXT: exit:
+; CHECK-NEXT: middle.block:
 ; CHECK-NEXT: No successors
 ; CHECK-NEXT: }
 entry:
   br label %outer.header
 
 outer.header:
   %outer.iv = phi i64 [ 0, %entry ], [ %outer.iv.next, %outer.latch ]
   %gep.1 = getelementptr inbounds [8 x i64], ptr @arr2, i64 0, i64 %outer.iv

llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp

 compound=true
   N0 [label =
     "ph:\l" +
     "  EMIT vp\<%1\> = EXPAND SCEV (-1 + %N)\l" +
     "No successors\l"
   ]
   N1 [label =
     "vector.ph:\l" +
-    "Successor(s): for.body\l"
+    "Successor(s): vector loop\l"
   ]
   N1 -> N2 [ label="" lhead=cluster_N3]
   subgraph cluster_N3 {
     fontname=Courier
-    label="\<x1\> for.body"
+    label="\<x1\> vector loop"
     N2 [label =
       "vector.body:\l" +
       "  WIDEN-PHI ir\<%indvars.iv\> = phi ir\<0\>, ir\<%indvars.iv.next\>\l" +
       "  EMIT ir\<%arr.idx\> = getelementptr ir\<%A\>, ir\<%indvars.iv\>\l" +
       "  EMIT ir\<%l1\> = load ir\<%arr.idx\>\l" +
       "  EMIT ir\<%res\> = add ir\<%l1\>, ir\<10\>\l" +
       "  EMIT store ir\<%res\>, ir\<%arr.idx\>\l" +
       "  EMIT ir\<%indvars.iv.next\> = add ir\<%indvars.iv\>, ir\<1\>\l" +
       "  EMIT ir\<%exitcond\> = icmp ir\<%indvars.iv.next\>, ir\<%N\>\l" +
       "  EMIT branch-on-cond ir\<%exitcond\>\l" +
       "No successors\l"
     ]
   }
   N2 -> N4 [ label="" ltail=cluster_N3]
   N4 [label =
-    "for.end:\l" +
+    "middle.block:\l" +
     "No successors\l"
   ]
 }
 )";
   EXPECT_EQ(ExpectedStr, FullDump);
 #endif
   TargetLibraryInfoImpl TLII(Triple(M.getTargetTriple()));
   TargetLibraryInfo TLI(TLII);