[llvm] [VPlan] Introduce scalar loop header in plan, remove VPLiveOut. (PR #109975)
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Sun Oct 13 12:18:27 PDT 2024
fhahn wrote:
> > Sure. Curious if you hit any limitations with VPIRInstruction that prevent it from replacing VPLiveOut?
>
> Yeah, so in my comment earlier last week I highlighted the problem of ordering constraints. If the same exit block is used by both the latch and the early exit, then the phi in the exit block should have an incoming value for both of the exiting blocks. If I add operands to the VPIRInstruction wrapper - one for each exiting block - then in the VPIRInstruction::execute code using a patch you sketched out in [3cfe25b](https://github.com/llvm/llvm-project/commit/3cfe25bf43feaf94d319c7f804e5c73c45f96057) we introduce a requirement that the order of the extra operands _must_ match the order of the predecessors. `replaceVPBBWithIRVPBB` breaks this by reordering them, hence I put up a patch yesterday to preempt this - #111514. Perhaps it's fine to introduce this ordering constraint, but I wonder if this makes the code fragile and prone to breaking every time we do some CFG optimisation?
The order of successors/predecessors is already meaningful in VPlan, it is e.g. also used determine to true/false successors for branches. So far, `replaceVPBBWithIRVPBB` wasn't an issue, because of the middle vpbb only have a single predecessor. Ensuring that more generally via #111514 seems like the right way to go, thanks for putting up the patch.
> Secondly, the phi may or may not reference an induction variable in the loop, which is handled separately to VPIRInstruction::execute. However, you still need to add some sort of dummy operand because the number of operands _must_ match the number of predecessors. I have hacked this downstream by adding a static 'null' VPValue marker that we can skip over the operand in VPIRInstruction::execute.
>
Yes, fixing induction users in the exit block is the last remaining exit users handling not managed by VPlan, but this should also be migrated soon. I shared a stack of patches that move computing the end value to VPlan (https://github.com/llvm/llvm-project/pull/112145) and updating the exit users of IVs, retiring fixupIVUsers (https://github.com/llvm/llvm-project/pull/112147). They both depend on this patch and https://github.com/llvm/llvm-project/pull/110577, but should give a good idea on what is needed to complete the transition of modeling all live-outs directly in VPlan.
https://github.com/llvm/llvm-project/pull/109975
More information about the llvm-commits
mailing list