[PATCH] D73078: [VPlan] Use consecutive numbers to print VPValues instead of addresses.

[Gil Rapaport via Phabricator via llvm-commits]

gilr added a comment.

In D73078#1900122 <https://reviews.llvm.org/D73078#1900122>, @fhahn wrote:

> Updated patch to use getPlan(), add unit tests.
>
> In D73078#1899992 <https://reviews.llvm.org/D73078#1899992>, @gilr wrote:
>
> > In D73078#1870844 <https://reviews.llvm.org/D73078#1870844>, @fhahn wrote:
> >
> > > [snip]
> > >  I've put up a patch to add a pointer to VPlan: D74445 <https://reviews.llvm.org/D74445>. I;ve also added implementations of print() with and without SlotTracker (so we can re-use the existing tracker, e.g. from VPlanPrinter. I still need to update the slot tracker to actually use it. VPInstructions without a block (or a VPBasicBlock without parentPlan) should probably be handled similar to IR instructions without BB, by using something like 'badref'.
> > >
> > > What do you think?
> >
> >
> > This added pointer to VPlan can indeed take care of enumerating VPInstructions, defaulting to 'badref'.
> >  Essentially all other VPValues except BackedgeTakenCount are wrappers of underlying IR values (right?), so they can print that value e.g. with the proposed `%ir.` w/o needing SlotTracker's services, thereby simplifying the patch. This would leave the somewhat special BackedgeTakenCount as the only user of the "where:" clause and current pointer-based naming.
>
>
> I think we also introduce some VPInstructions not connected to any underlying IR during predication and will add more in the future. I think we should do (and need) both, using the IR name when available and the slot tracker otherwise. I think it would be good to have using the underlying IR names as a follow up to this patch. What do you think?


After D74445 <https://reviews.llvm.org/D74445> all VPInstructions can indeed print themselves by either using their IR name or defaulting to SlotTracker/badref. So this patch can be simplified by not propagating the SlotTracker through the print() methods, since non-VPInstruction VPValues can rely on their underlying IR values for printing. Note that for this to work addVPValue must plant the underlying IR value into the generated VPValue as you do in D74695 <https://reviews.llvm.org/D74695>.
BackedgeTakenCount would then remain the only user of the current pointer-based naming and consequently of the "where:" clause.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73078