llvm dev - May 2018 - Can creating new forms of debug info metadata be simplified? [formatting fixed]

Thanks all for your response.

On Tue, May 29, 2018, at 5:38 PM, Duncan P. N. Exon Smith
wrote:
> 
> 
>> On May 29, 2018, at 12:55, Adrian Prantl <aprantl at apple.com>
wrote:
>> 
>> 
>> 
>>> On May 29, 2018, at 12:28 PM, David Blaikie <dblaikie at
gmail.com> wrote:
>>> 
>>> +some of the debug info cabal (& Duncan, as an emeritus member,
and person who plumbed a lot of the current debug info syntax support in)
>>> 
>>> Visitor seems plausible though I haven't looked at the code in
detail to see if it'd work perfectly.
>>> 
>>> 
> SGTM!  There was a hope (not quite a plan) that these would eventually be
tablegen'ed, but visitor sounds fine to me too.
> 
>> Something (anything) along these lines seems like a good idea to me. In
addition to the cost of adding new nodes, having less repetitive
manually-written existing code reduces the chances for bugs and increases
readability. There are some irregularities in the existing code that I'm
aware of that would need to be still handled separately:
>> - The MDNodeKeyImpl currently is manually tuned to only hash members
that are likely to differ.
>> - The deserialization code also supports various older serialization
formats.
> 
> These are the two corner cases I was thinking of.
I had these ones in mind as well and my experience using the template visitor
pattern for this kind of thing is that the API can usually be marked to add that
metadata (metadata-ception).

For example, the concept of members necessary for uniquifying an instance via a
hash will map nicely to the concept of "composite primary keys" which
has a clean solution that I like[1], and could be used as inspiration. Or even
something as simple as:

template<typename Visitor>
void visit(Visitor & v) {
   v.keys("Property1","Property2","Property3");
}

I have never done this, but I expect we could also use constexpr to unroll many
things at compile-time. That could be fun!

Backward compatibility for serialization is also achievable using a method very
similar to Boost serialization[2]:

template<typename Visitor>
void visit(Visitor & v) {
   v.keys("Version1Property");
   if(v.dbgInfoVersion >= 2) // always the latest when serializing out
      v.property("Version2Property",Version2Property);
   if(v.dbgInfoVersion >= 1) // otherwise set to the version when reading
back in
      v.property("Version1Property","Version1Property");
}

If either of the above two use cases don't work out, TableGen'ing is a
viable option as well.

Anything sound completely out of whack here? Any other use cases?

[1]
https://www.webtoolkit.eu/wt/doc/reference/html/structWt_1_1Dbo_1_1dbo__traits.html
[2]
https://www.boost.org/doc/libs/1_67_0/libs/serialization/doc/tutorial.html#versioning
> 
>>>> 
>>>> Thanks for your time!
>>>> 
>>>> --
>>>> Sohail Somani
>>>> Fizz Buzz Inc.
>>>> Booking schedule: https://sohailsomani.youcanbook.me
>>>> 
>>>> _______________________________________________
>>>> LLVM Developers mailing list
>>>> llvm-dev at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>

> On May 29, 2018, at 15:33, Sohail Somani (Fizz Buzz Inc.) <sohail at
fizzbuzzinc.com> wrote:
> 
> Thanks all for your response.
> 
> On Tue, May 29, 2018, at 5:38 PM, Duncan P. N. Exon Smith wrote:
>> 
>> 
>>> On May 29, 2018, at 12:55, Adrian Prantl <aprantl at
apple.com> wrote:
>>> 
>>> 
>>> 
>>>> On May 29, 2018, at 12:28 PM, David Blaikie <dblaikie at
gmail.com> wrote:
>>>> 
>>>> +some of the debug info cabal (& Duncan, as an emeritus
member, and person who plumbed a lot of the current debug info syntax support
in)
>>>> 
>>>> Visitor seems plausible though I haven't looked at the code
in detail to see if it'd work perfectly.
>>>> 
>>>> 
>> SGTM!  There was a hope (not quite a plan) that these would eventually
be tablegen'ed, but visitor sounds fine to me too.
>> 
>>> Something (anything) along these lines seems like a good idea to
me. In addition to the cost of adding new nodes, having less repetitive
manually-written existing code reduces the chances for bugs and increases
readability. There are some irregularities in the existing code that I'm
aware of that would need to be still handled separately:
>>> - The MDNodeKeyImpl currently is manually tuned to only hash
members that are likely to differ.
>>> - The deserialization code also supports various older
serialization formats.
>> 
>> These are the two corner cases I was thinking of.
> 
> I had these ones in mind as well and my experience using the template
visitor pattern for this kind of thing is that the API can usually be marked to
add that metadata (metadata-ception).
> 
> For example, the concept of members necessary for uniquifying an instance
via a hash will map nicely to the concept of "composite primary keys"
which has a clean solution that I like[1], and could be used as inspiration. Or
even something as simple as:
> 
> template<typename Visitor>
> void visit(Visitor & v) {
>  
v.keys("Property1","Property2","Property3");
> }
> 
> I have never done this, but I expect we could also use constexpr to unroll
many things at compile-time. That could be fun!
> 
> Backward compatibility for serialization is also achievable using a method
very similar to Boost serialization[2]:
> 
> template<typename Visitor>
> void visit(Visitor & v) {
>   v.keys("Version1Property");
>   if(v.dbgInfoVersion >= 2) // always the latest when serializing out
>      v.property("Version2Property",Version2Property);
>   if(v.dbgInfoVersion >= 1) // otherwise set to the version when reading
back in
>      v.property("Version1Property","Version1Property");
> }
As a heads up, bitcode records tend to be versioned somewhat implicitly based on
their structure (e.g., number of fields).
> 
> If either of the above two use cases don't work out, TableGen'ing
is a viable option as well.
Or we can leave some code written explicitly (like reading bitcode, which seems
the hardest to deal with).
> Anything sound completely out of whack here? Any other use cases?
> 
> [1]
https://www.webtoolkit.eu/wt/doc/reference/html/structWt_1_1Dbo_1_1dbo__traits.html
> [2]
https://www.boost.org/doc/libs/1_67_0/libs/serialization/doc/tutorial.html#versioning
> 
>> 
>>>>> 
>>>>> Thanks for your time!
>>>>> 
>>>>> --
>>>>> Sohail Somani
>>>>> Fizz Buzz Inc.
>>>>> Booking schedule: https://sohailsomani.youcanbook.me
>>>>> 
>>>>> _______________________________________________
>>>>> LLVM Developers mailing list
>>>>> llvm-dev at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>

On Tue, May 29, 2018, at 7:41 PM, Duncan P. N. Exon Smith
wrote:
> As a heads up, bitcode records tend to be versioned somewhat implicitly 
> based on their structure (e.g., number of fields).
Yes, this is true. You want to do that to reduce space usage. Intuitively, I
sense that we can work around this using a similar logic but it's something
I'd have to think about some more. Alternatively, we could add some kind of
a prefix field for the tag version.
> > If either of the above two use cases don't work out,
TableGen'ing is a viable option as well.
> 
> Or we can leave some code written explicitly (like reading bitcode, 
> which seems the hardest to deal with).
I think if this were to be done, v1 would be designed to handle the 80% use
case, with an escape hatch to use the current approach. So the complex cases
(there is some hairy stuff) would continue to be written as is today.

Still, I don't want anyone to voluntarily have to write/read bitcode again,
myself included, or perhaps especially myself :)

Sohail