This patch introduces a helper to obtain an iterator range for the
PHI-like recipes in a block.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
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 | ||
---|---|---|
1485 | worth adding getFirstNonPHI() and have phis() use it? | |
1491 | 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. |
Updates:
- add comment about PHI node order to VPBasicBlock
- use existing getFirstNonPhi() in phis()
- add VPrecipeBase::isPhi(), use it in getFirstNonPhi
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?
llvm/lib/Transforms/Vectorize/VPlan.h | ||
---|---|---|
1485 | There's already a getFirstNonPhi, which I originally missed. I updated the code to use it directly (and moved phis() to getFirstNonPhi. | |
1491 |
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? |
worth adding getFirstNonPHI() and have phis() use it?