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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1477
   inline VPRecipeBase &back() { return Recipes.back(); }
 
+  /// Returns an iterator range over the PHI-like recipes in the block.
----------------
Ayal wrote:
> worth adding getFirstNonPHI() and have phis() use it?
There's already a `getFirstNonPhi`, which I originally missed. I updated the code to use it directly (and moved `phis()` to `getFirstNonPhi`.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1483
+      VPRecipeBase *R = &*FirstNonPhi;
+      if (!isa<VPWidenIntOrFpInductionRecipe>(R) && !isa<VPWidenPHIRecipe>(R))
+        break;
----------------
Ayal wrote:
> 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.
> 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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100101