[PATCH] IR: Move the slot tracker out of AsmWriter into a separate public module.

[Alex L]

2015-06-17 14:15 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:

>
> > On 2015 Jun 17, at 14:03, Alex L <arphaman at gmail.com> wrote:
> >
> >
> >
> > 2015-06-17 13:52 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com
> >:
> >
> > > On 2015 Jun 17, at 13:20, Alex Lorenz <arphaman at gmail.com> wrote:
> > >
> > > Hi dexonsmith, bob.wilson, bogner,
> > >
> > > This patch moves the SlotTracker class out of AsmWriter.cpp into a
> separate module that's publicly accessible.
> > >
> > > This patch would be useful for MIR Serialization, in particular it
> would enable the MIR parser to parse metadata machine operands. The
> metadata machine operands are serialized using the familiar '!' <slot>
> notation, and the MIR parser has to be able to map from slot numbers to the
> actual metadata nodes. The SlotTracker class would allow the MIRParser to
> create this mapping.
> >
> > I can see that this would be useful for *writing* .mir files, but I
> > don't think you can safely use this for *reading* .mir files.
> >
> > Metadata slots can be assigned arbitrarily in an LLVM IR file, such as:
> >
> >     !named = !{!36, !72}
> >     !72 = !{!"string"}}
> >     !36 = !{!72, !{!{}}}
> >
> > If you were to parse the module and then run the slot tracker, you'd get:
> >
> >     !named = !{!1, !2}
> >     !1 = !{!2, !3}
> >     !2 = !{!"string"}
> >     !3 = !{!4}
> >     !4 = !{}
> >
> > or something close to that.
> >
> > You couldn't safely take an already-parsed Module, run the slot-tracker
> > on it, and then parse machine functions that referenced metadata.  But
> > it sounds like that's what you're suggesting?
> >
> > This makes sense, yeah this patch wouldn't really work then.
> >
> >
> > Instead, I think you need to:
> >
> >  1. Yes, surface the slot tracker (exactly this patch), but for
> completely
> >     different reasons:  so that you can write out correct metadata
> numbers
> >     for metadata references within machine functions, to match the
> metadata
> >     that you wrote out for the LLVM IR.
> >  2. Use (1) so that the same slots are used when writing LLVM IR portion
> of
> >     MIR as the machine functions.
> >
> > I don't need to surface the slot tracker then, as I can print out the
> correct metadata slot numbers by printing the metadata nodes as operands.
> They create the slot tracker and initialize it for the whole module, so the
> correct slot numbers are printed.
>
> Oof, that sounds expensive.  Actually, I know it is: I made it expensive,
> since previously it was just about useless :).
>

Yeah, it is expensive right now, but this patch can be resubmitted later
along with a follow up patch that allows you to print metadata nodes when
you have a slot tracker. Then we can just modify the MIR printing code to
make it efficient.

I don't think this is the only case where this situation happens, I think
some of the other LLVM value printing cases can be more efficient as well.


>
> This'll be fine for hand-written testcases, but if someone is debugging
> some crash from a big input, the MIR will take O(N^2) to print (needs to
> make slots for all metadata every time something prints out a metadata
> machine operand).
>
> Given that the main goal right now *is* testcases, maybe this is okay,
> but please add a FIXME to make it efficient as a follow-up.
>

Sure.


>
> >
> >  3. Find a way to surface `LLParser::NumberedMetadata` in a similar way
> >     (could be some way of copying them out?) when parsing the LLVM IR
> part
> >     of MIR.
> >  4. Use that slot mapping when you're reading machine functions.
> >
> > Yeah, I should surface the slot mapping from the LLParser then.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150617/5ce0d935/attachment.html>