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

Tue Mar 3 04:35:19 PST 2020

fhahn added a comment.

In D73078#1902664 <https://reviews.llvm.org/D73078#1902664>, @gilr wrote:

> > [snip]
> >  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.

Ah I think I got what you are proposing now: just remove the `::print(raw_ostream &O, const Twine &Indent, VPSlotTracker &SlotTracker)` variants taking a SlotTracker, because we can just instantiate it on demand, by just getting the parent plan for the containing VPBB. This means we might have to re-number the plan multiple times when printing the whole plan through VPlanPrinter, but as this is just for debugging (and the scope of a plan should be relatively small) we can adjust that as follow-up, if required.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73078