[VPlan] Add VPBasicBlock::phis() helper (NFC).

[fhahn]

Summary

This patch introduces a helper to obtain an iterator range for the
PHI-like recipes in a block.

[fhahn]

ping :)

[Ayal]

Looks good to me!

This is aligned with BasicBlock::phis(), with a simpler implementation based on finding FirstNonPHI. Number of phi's is expected to be limited, so running twice until FirstNonPHI instead of once should be tolerable.

Would be good to document some place that all phi recipes appear before any non-phi recipe in VPBasicBlock, as in BasicBlock. (This assumption predates this patch.)

llvm/lib/Transforms/Vectorize/VPlan.h

1477

worth adding getFirstNonPHI() and have phis() use it?

1483

worth having both these recipes inherit from a common base class, which represents all phi recipes, resulting in a single isa<> here?

once some of these header phi recipes are converted into VPInstructions, this may be more complicated.

[fhahn]

Updates:

1. add comment about PHI node order to VPBasicBlock
2. use existing getFirstNonPhi() in phis()
3. add VPrecipeBase::isPhi(), use it in getFirstNonPhi

In D100101#2718833, @Ayal wrote:

Looks good to me!

This is aligned with BasicBlock::phis(), with a simpler implementation based on finding FirstNonPHI. Number of phi's is expected to be limited, so running twice until FirstNonPHI instead of once should be tolerable.

Would be good to document some place that all phi recipes appear before any non-phi recipe in VPBasicBlock, as in BasicBlock. (This assumption predates this patch.)

Thanks for taking a look! I added a sentence to the top-level comment for VPBasicBlock saying that all PHI-like recipes must come before non-phi recipes. I hope that's sufficient or were you thinking about a different place?

[fhahn]

llvm/lib/Transforms/Vectorize/VPlan.h

1477

There's already a getFirstNonPhi, which I originally missed. I updated the code to use it directly (and moved phis() to getFirstNonPhi.

1483

worth having both these recipes inherit from a common base class, which represents all phi recipes, resulting in a single isa<> here?

I'm not sure if there is much benefit at the moment, as I don't think the PHI-like recipes could really share much in the common base class?

I added a VPRecipeBase::isPhi() helper to directly check if a recipe is PHI-like, so we have a single place for such a check. I think that should give us the same convenience with respect to having a simple check for PHI-like recipes, but without having to add an extra level in the class hierarchy for now. WDYT?

[fhahn]

Rebased so this now applies directly on top of main.

[fhahn]

Ping :)

[Ayal]

This is fine, thanks!

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

 void InnerLoopVectorizer::fixCrossIterationPHIs(VPTransformState &State) {
   // In order to support recurrences we need to be able to vectorize Phi nodes.
   // Phi nodes have cycles, so we need to vectorize them in two stages. This is
   // stage #2: We now need to fix the recurrences by adding incoming edges to
   // the currently empty PHI nodes. At this point every instruction in the
   // original loop is widened to a vector form so we can use them to construct
   // the incoming edges.
   VPBasicBlock *Header = State.Plan->getEntry()->getEntryBasicBlock();
-  for (VPRecipeBase &R : *Header) {
-    if (isa<VPWidenIntOrFpInductionRecipe>(&R))
-      continue;
+  for (VPRecipeBase &R : Header->phis()) {
     VPWidenPHIRecipe *PhiR = dyn_cast<VPWidenPHIRecipe>(&R);
     if (!PhiR)
-      break;
-
+      continue;
     auto *OrigPhi = cast<PHINode>(PhiR->getUnderlyingValue());
     if (PhiR->getRecurrenceDescriptor()) {
       fixReduction(PhiR, State);
     } else if (Legal->isFirstOrderRecurrence(OrigPhi))
       fixFirstOrderRecurrence(OrigPhi, State);
   }
 }
 

llvm/lib/Transforms/Vectorize/VPlan.h

public:
   inline const_reverse_iterator rend() const { return Recipes.rend(); }
 
   inline size_t size() const { return Recipes.size(); }
   inline bool empty() const { return Recipes.empty(); }
   inline const VPRecipeBase &front() const { return Recipes.front(); }
   inline VPRecipeBase &front() { return Recipes.front(); }
   inline const VPRecipeBase &back() const { return Recipes.back(); }
   inline VPRecipeBase &back() { return Recipes.back(); }
 
+  /// Returns an iterator range over the PHI-like recipes in the block.
+  iterator_range<iterator> phis() {
+    auto FirstNonPhi = begin();
+    for (auto E = end(); FirstNonPhi != E; ++FirstNonPhi) {
+      VPRecipeBase *R = &*FirstNonPhi;
+      if (!isa<VPWidenIntOrFpInductionRecipe>(R) && !isa<VPWidenPHIRecipe>(R))
+        break;
+    }
+
+    return make_range(begin(), FirstNonPhi);
+  }
+
   /// Returns a reference to the list of recipes.
   RecipeListTy &getRecipeList() { return Recipes; }
 
   /// Returns a pointer to a member of the recipe list.
   static RecipeListTy VPBasicBlock::*getSublistAccess(VPRecipeBase *) {
     return &VPBasicBlock::Recipes;
   }