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

[Mehdi AMINI via llvm-commits]

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/5ea4fbf9/attachment-0001.html>