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

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 244008.
fhahn added a comment.

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

> In D73078#1855736 <https://reviews.llvm.org/D73078#1855736>, @fhahn wrote:
>
> > In D73078#1854151 <https://reviews.llvm.org/D73078#1854151>, @gilr wrote:
> >
> > > It would indeed be good to improve VPValue printing. Initially, there were only a handful of VPValues so those pointer-based IDs seemed sufficient.
> > >  Since VPValues are now both more common and optionally have an underlying IR value, retaining names from the underlying IR where available could go a long way w.r.t both readability and relating to the loop being vectorized; This would hopefully cover most VPValues.
> >
> >
> > Sounds good, printing the name of the underlying value should be quite trivial.
> >
> > > BTW, VP basic blocks could also retain the name of the corresponding IR basic block.
> >
> > Sounds good!
> >
> > > It would also be good to distinguish between "pure" VPValues and VPValues wrapping unnamed IR values (can we somehow retain the latter's enumeration?)
> >
> > +1. I think we would still need a mechanism like this patch to provide a better numbering for 'pure' VPValues.
>
>
> Agreed.
>
> > For VPValues with unnamed wrapped IR values, I think it should be possible to print the number in the IR ( this will implicitly create an IR slot tracker and number the function on demand). We could use a scheme similar to machine IR and use something like `%ir.`(IR name)  for VPValues with underlying IR values.
> > 
> > What do you think?
>
> On second thought, retaining original IR names and/or numbers is probably worth a separate discussion, also concerning names generated on VPlan execution.
>  The proposed Slot Tracker seems like a good foundation to have and possibly extend later, e.g. facilitating giving (enumerated) meaningful names to VPInstructions and retaining original IR name/number of live-ins.
>
> Adding a pointer to VPlan in VPBlockBase would indeed be less intrusive to the code; how would this work for VPInstructions not assigned to any block?


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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73078

Files:
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/lib/Transforms/Vectorize/VPlan.cpp
  llvm/lib/Transforms/Vectorize/VPlan.h
  llvm/lib/Transforms/Vectorize/VPlanValue.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73078.244008.patch
Type: text/x-patch
Size: 17406 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200211/314bde44/attachment.bin>