[PATCH] D34080: [ThinLTO] Add dump-summary command to llvm-lto2 tool

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 12 13:52:51 PDT 2017


I'm not sure why there's a concern that coming to consensus on a reasonable
format for this might take a long time.

Has there been a history of that? The last few IR changes I recall (use
lists, comdats) don't seem like they had a lot of debate over the format,
did they?

- Dave

On Mon, Jun 12, 2017 at 1:50 PM Mehdi AMINI <joker.eph at gmail.com> wrote:

> 2017-06-12 13:04 GMT-07:00 Chandler Carruth <chandlerc at gmail.com>:
>
>> On Mon, Jun 12, 2017 at 12:59 PM David Blaikie <dblaikie at gmail.com>
>> wrote:
>>
>>> On Mon, Jun 12, 2017 at 12:20 PM Peter Collingbourne <peter at pcc.me.uk>
>>> wrote:
>>>
>>>> On Mon, Jun 12, 2017 at 12:16 PM, David Blaikie <dblaikie at gmail.com>
>>>> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Mon, Jun 12, 2017 at 12:13 PM Peter Collingbourne <peter at pcc.me.uk>
>>>>> wrote:
>>>>>
>>>>>> I think we should start with this patch so that we aren't forced to
>>>>>> go further in what I hope is clearly the wrong direction.
>>>>>>
>>>>>
>>>>> I'm not sure what you mean - it feels to me like this patch is "going
>>>>> further in what is clearly the wrong direction" - a direction that's adding
>>>>> workarounds by providing dumping features instead of roundtrippable .ll
>>>>> syntax?
>>>>>
>>>>
>>>> I don't necessarily agree that this is the wrong direction or a
>>>> workaround, which is why I think we should discuss it in parallel.
>>>>
>>>
>>> This has already slipped further than is ideal - when bitcode support
>>> was added for summaries, .ll support should've been provided at the same
>>> time, as with any other bitcode feature in LLVM. This is a baseline
>>> requirement for features in LLVM's bitcode that seems to have been missed
>>> during the initial support/code review.
>>>
>>> When YAML support was added for reading summaries, that would've been a
>>> great opportunity to go back and address this missing functionality.
>>>
>>> Now that dumping support is needed/being added, there's another
>>> opportunity to go in that direction rather than continuing to work further
>>> away from it.
>>>
>>
>> FWIW, just as textual serialization/de-serialization as part of the
>> textual IR was a critical component of preserving use-list order (in
>> addition to the bitcode support), I think the same is true for summaries.
>>
>> I'm perfectly happy to keep them *semantically* distinct from the IR (and
>> in a distinct in-memory representation) for now, as that really is a
>> separate concern as Mehdi has pointed out in other contexts.
>>
>> But the basic properties of serialization/de-serialization, being able to
>> round trip, and write self-contained test cases should always be common
>> between bitcode and textual IR. The fact that summaries have drifted here
>> should be corrected sooner rather than later, and insisting upon that
>> before going further down the path seems a very reasonable step to ensure
>> we get the right thing long-term.
>>
>
> I see two main paths:
>
> 1) We iron down the right final format for the .ll and implement it. In
> the meantime we don't have a good flow to write tests with summaries.
> 2) We already have a serialization/deserialization in YAML, why not using
> it as a starting point? That was introduced to be able to write testing
> without depending on getting the right final format to achieve `llvm-dis |
> llvm-as` (i.e. Peter didn't want to hammer down the right .ll format or
> wait for it).  We could use this to "bootstrap" by injecting it in the .ll
> somehow, and progress as incremental steps from there to a better .ll
> format. Knowing that the YAML is machine readable and so can be upgraded to
> whatever futur .ll format, so we'll be able to convert to it automatically
> and there isn't much debt involved.
>
> Unless someone wants to drive very actively 1), it seems pragmatic to me
> to go with 2) to not block/hinder progress on getting better testing on
> LTO/ThinLTO summaries.
>
> --
> Mehdi
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170612/87414147/attachment.html>


More information about the llvm-commits mailing list