[PATCH] D95382: [VPlan] Make VPBlockBase a VPUser.

Mon Feb 1 07:47:43 PST 2021

fhahn added a comment.

In D95382#2528700 <https://reviews.llvm.org/D95382#2528700>, @a.elovikov wrote:

> Hi Florian, I'm starting to familiarize myself with VPlan and wonder if that is a short/mid-term solution or something that is expected to be a long-term.

Hi, that's great!

> Have you considered modeling the edges using terminator instructions similar to what regular LLVM IR does? I see the following potential advantages:
>
> 1. More straightforward thinking about dominance relation/theoretical conception. CondBit might be defined inside the VPBB itself and in order to have that use not to break the dominance we need the VPBB "defined" at its end, which might be slightly confusing. I'm not sure if that can lead to any practical difficulties though.
> 2. By having a dedicated terminator we can put some special semantics/operation on the condition into that terminator. For example, VPBranchInst instruction for a regular branch on a uniform condition and something like VPBranchIfAllZero/VPBranchIfAllOnes(mask) when condition is varying. Of course, the same can be encoded into the VPBB itself, but the explicit instruction might be considered slightly more readable.
> 3. Potential simplification of successors/predecessors handling - that will be done by the def/use edges.
>
> A big disadvantage is, obviously, that the change would be more invasive and the benefits aren't really too "objective". Still, it might be worth considering this direction.
>
> What do you think?

Modeling terminators explicitly would certainly make a few things a bit nicer.

But I think in the short term, the amount of churn required to change that would not be worth it. The CondBit use in VPBlockBase is one of the few remaining cases not in the VPlan def-use chains and I think it would be preferable to get this wrapped up ASAP and re-visit the issue of explicit terminators in the future.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95382