[llvm-dev] [RFC][ThinLTO] llvm-dis ThinLTO summary dump format

[David Blaikie via llvm-dev]

On Wed, Jun 7, 2017 at 12:09 PM Mehdi AMINI <joker.eph at gmail.com> wrote:

> 2017-06-07 11:06 GMT-07:00 David Blaikie <dblaikie at gmail.com>:
>
>> I don't think this represent my position. I mentioned earlier in the
>>> thread that we should have "llvm-dis | llvm-as" being able to round-trip
>>> bitcode, including summaries when present.
>>>
>>
>> OK, then I think there's been a lot of confusion, so maybe focussing on
>> these comments might be more useful.
>>
>> How do you envision that happening? That the summary would be written in
>> the textual representation?
>>
>
> Yes, I don't think anyone in the thread argued against this. The original
> proposal was not accounting for reading them back though.
>
>
>> (or it could be reconstructed, as you mention, by having a single
>> bit/flag/something in the textual representation that says "this has a
>> summary" & then a summary could be generated from the Module and written
>> out to the bitcode)
>>
>
>> That's all I'm really interested in in this discussion. That summaries
>> seem like they should be as well represented in .ll files as they are in
>> .bc files.
>>
>
> Right.
>
>
>>
>> And if that's the case, it seems weird for them to be YAML - YAML in a
>> .ll file would seem out of place/inconsistent with the rest of what's in a
>> .ll file, to me. Not impossible, but a bit 'off'.
>>
>
> I agree, and this was raised as a concern in the first place when the YAML
> format was introduced. But IIRC Peter pushed for the YAML as an
> intermediate easy step to get results, and advocated for punting nailing
> down an IR format to later.
>

*nod* Thanks for bringing it up then & I'm sorry I missed it to
support/agree with you there. So I'm raising it now and advocate for that
"later" being now.

Doesn't mean this patch should fix everything right now, but I think this
is probably a point worth having the conversation about what the desired
end state is and how patches help get there (or don't - it's OK to decide
it won't be solved immediately and some intermediate solution is desirable,
but I'd like to understand why as that's not usually been the case for
other features added to the .ll/.bc format)

May be worth an llvm-dev discussion/proposal "here's the desired syntax" &
a possible path to implementing that functionality. (& I'd see implementing
dumping in a nice format (maybe even what's being proposed here, but not in
comments - in the .ll file/syntax directly and in a syntax that looks
consistent with the rest of the contents of .ll files - keep it behind a
flag that can be used in test cases immediately), then parsing, then
migrating off the YAML based tests - or start with switching YAML input
tests to the new input format before there's dumping support, then add
dumping support, either way seems OK by me)


> Now the proposal in this thread is to dump summaries with llvm-dis, which
> I agree with, but what I raised in this thread is that we should be able to
> read them back. I also mentioned that we shouldn't keep *two* separate
> textual formats.
> As a first step, having `llvm-dis | llvm-as` using the YAML format seems
> like a pragmatic way of getting it done. I'd be happy if anyone wants to
> replace the YAML format with a more "integrated way" in the textual IR
> format, I didn't feel like it was worth blocking getting the feature in
> `llvm-dis` though.
>
> --
> Mehdi
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170608/b417e7b9/attachment.html>