llvm dev - May 2016 - RFC: metadata attachments for global variables

On Mon, May 9, 2016 at 1:53 PM, Peter Collingbourne <peter at pcc.me.uk>
wrote:
>
>
> On Fri, May 6, 2016 at 2:10 PM, David Blaikie <dblaikie at gmail.com>
wrote:
>
>>
>>
>> On Fri, May 6, 2016 at 2:09 PM, David Blaikie <dblaikie at
gmail.com> wrote:
>>
>>>
>>>
>>> On Fri, May 6, 2016 at 1:55 PM, Peter Collingbourne via llvm-dev
<
>>> llvm-dev at lists.llvm.org> wrote:
>>>
>>>>
>>>>
>>>> On Fri, May 6, 2016 at 1:21 PM, Duncan P. N. Exon Smith <
>>>> dexonsmith at apple.com> wrote:
>>>>
>>>>>
>>>>> > On 2016-May-06, at 13:17, Peter Collingbourne
<peter at pcc.me.uk>
>>>>> wrote:
>>>>> >
>>>>> > Hi all,
>>>>> >
>>>>> > I'd like to add support for metadata attachments
for global
>>>>> variables in the same way as we did for functions.
>>>>> >
>>>>> > Syntax would be pretty simple:
>>>>> > @foo = global i32 0, !foo !0, !bar !1
>>>>> > (the extra commas are required to disambiguate from a
named metadata
>>>>> on the next line)
>>>>>
>>>>> SGTM.
>>>>>
>>>>> > Benefits:
>>>>> > 1) Lets us reverse the DIGlobalVariable ->
GlobalVariable edge,
>>>>> which should hopefully clear the way for removing the
llvm.dbg.cu
>>>>> named metadata node.
>>>>>
>>>>> A little harder than it sounds (need to somehow support a
>>>>> GlobalVariable that is RAUW'ed with a ConstantInt), but
I think this is
>>>>> important to do.
>>>>>
>>>>
>>>> How can a GlobalVariable be replaced with a ConstantInt?
Wouldn't that
>>>> just be an absolute address?
>>>>
>>>
>>> If the global variable has a known constant value & the address
is never
>>> needed, then we might optimize away the storage and still want
debug info
>>> describing the constant.
>>>
>>
>> I'm not sure under what conditions, if any, that currently works -
so
>> this may be more of a "nice to have" or "be good to
think about how any
>> future design wouldn't preclude fixing this gap" sort of
thing.
>>
>
> It looks like this is how debug info for static data members used to look
> until r172591 landed back in 2013.
>
Could you explain more what you mean by "<this> is how it worked
until" -
'this' being how it used to work and how you're proposing to make it
work?
I'm not quite following.

> There are a couple of tests that exercise this path
> (DebugInfo/X86/multiple-at-const-val.ll and DebugInfo/X86/pr12831.ll) but
> it seems likely that they were just upgraded manually.
>
> Do we care about backwards compatibility for debug info from that far back?
>
Yeah, pretty sure we don't care.

> I assume we don't, since this predates the last debug info version bump
in
> 2015 (r231082 which introduced the debug info hierarchy). If that's the
> case, I intend to "upgrade" this metadata by dropping it, and we
can
> probably think of some other representation later for a constant folded
> global.
>
Upgrades to the verifier to fail when it sees this would be good too, most
likely. (maybe? maybe not... not sure)

>
> Thanks,
> --
> --
> Peter
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160509/1c940685/attachment.html>

On Mon, May 9, 2016 at 2:33 PM, David Blaikie <dblaikie at gmail.com>
wrote:
>
>
> On Mon, May 9, 2016 at 1:53 PM, Peter Collingbourne <peter at
pcc.me.uk>
> wrote:
>
>>
>>
>> On Fri, May 6, 2016 at 2:10 PM, David Blaikie <dblaikie at
gmail.com> wrote:
>>
>>>
>>>
>>> On Fri, May 6, 2016 at 2:09 PM, David Blaikie <dblaikie at
gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Fri, May 6, 2016 at 1:55 PM, Peter Collingbourne via
llvm-dev <
>>>> llvm-dev at lists.llvm.org> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Fri, May 6, 2016 at 1:21 PM, Duncan P. N. Exon Smith
<
>>>>> dexonsmith at apple.com> wrote:
>>>>>
>>>>>>
>>>>>> > On 2016-May-06, at 13:17, Peter Collingbourne
<peter at pcc.me.uk>
>>>>>> wrote:
>>>>>> >
>>>>>> > Hi all,
>>>>>> >
>>>>>> > I'd like to add support for metadata
attachments for global
>>>>>> variables in the same way as we did for functions.
>>>>>> >
>>>>>> > Syntax would be pretty simple:
>>>>>> > @foo = global i32 0, !foo !0, !bar !1
>>>>>> > (the extra commas are required to disambiguate
from a named
>>>>>> metadata on the next line)
>>>>>>
>>>>>> SGTM.
>>>>>>
>>>>>> > Benefits:
>>>>>> > 1) Lets us reverse the DIGlobalVariable ->
GlobalVariable edge,
>>>>>> which should hopefully clear the way for removing the
llvm.dbg.cu
>>>>>> named metadata node.
>>>>>>
>>>>>> A little harder than it sounds (need to somehow support
a
>>>>>> GlobalVariable that is RAUW'ed with a ConstantInt),
but I think this is
>>>>>> important to do.
>>>>>>
>>>>>
>>>>> How can a GlobalVariable be replaced with a ConstantInt?
Wouldn't that
>>>>> just be an absolute address?
>>>>>
>>>>
>>>> If the global variable has a known constant value & the
address is
>>>> never needed, then we might optimize away the storage and still
want debug
>>>> info describing the constant.
>>>>
>>>
>>> I'm not sure under what conditions, if any, that currently
works - so
>>> this may be more of a "nice to have" or "be good to
think about how any
>>> future design wouldn't preclude fixing this gap" sort of
thing.
>>>
>>
>> It looks like this is how debug info for static data members used to
look
>> until r172591 landed back in 2013.
>>
>
> Could you explain more what you mean by "<this> is how it worked
until" -
> 'this' being how it used to work and how you're proposing to
make it work?
> I'm not quite following.
>
Sorry, I meant that prior to r172591 we would represent a static data
member with something that looked like what a DIGlobalVariable looks like
today, but with a ConstantInt representing the initializer in the variable
field (which is where the GlobalVariable is normally stored). The change
moved the initializer to an associated "static member type" (yes,
it's not
really a type), which as a side effect caused us to (as far as I can tell)
no longer store ConstantInts in the variable field.

I'm not planning to change this aspect of the representation.

> There are a couple of tests that exercise this path
>> (DebugInfo/X86/multiple-at-const-val.ll and DebugInfo/X86/pr12831.ll)
but
>> it seems likely that they were just upgraded manually.
>>
>> Do we care about backwards compatibility for debug info from that far
>> back?
>>
>
> Yeah, pretty sure we don't care.
>
>
>> I assume we don't, since this predates the last debug info version
bump
>> in 2015 (r231082 which introduced the debug info hierarchy). If
that's the
>> case, I intend to "upgrade" this metadata by dropping it, and
we can
>> probably think of some other representation later for a constant folded
>> global.
>>
>
> Upgrades to the verifier to fail when it sees this would be good too, most
> likely. (maybe? maybe not... not sure)
>
This would no longer be representable in IR, as I'd be removing the
variable field from DIGlobalVariable as part of reversing the
DIGlobalVariable->GlobalVariable edge.

Thanks,
-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160509/ac41947c/attachment.html>

On Mon, May 9, 2016 at 3:17 PM, Peter Collingbourne <peter at pcc.me.uk>
wrote:
>
>
> On Mon, May 9, 2016 at 2:33 PM, David Blaikie <dblaikie at gmail.com>
wrote:
>
>>
>>
>> On Mon, May 9, 2016 at 1:53 PM, Peter Collingbourne <peter at
pcc.me.uk>
>> wrote:
>>
>>>
>>>
>>> On Fri, May 6, 2016 at 2:10 PM, David Blaikie <dblaikie at
gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Fri, May 6, 2016 at 2:09 PM, David Blaikie <dblaikie at
gmail.com>
>>>> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Fri, May 6, 2016 at 1:55 PM, Peter Collingbourne via
llvm-dev <
>>>>> llvm-dev at lists.llvm.org> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Fri, May 6, 2016 at 1:21 PM, Duncan P. N. Exon Smith
<
>>>>>> dexonsmith at apple.com> wrote:
>>>>>>
>>>>>>>
>>>>>>> > On 2016-May-06, at 13:17, Peter Collingbourne
<peter at pcc.me.uk>
>>>>>>> wrote:
>>>>>>> >
>>>>>>> > Hi all,
>>>>>>> >
>>>>>>> > I'd like to add support for metadata
attachments for global
>>>>>>> variables in the same way as we did for functions.
>>>>>>> >
>>>>>>> > Syntax would be pretty simple:
>>>>>>> > @foo = global i32 0, !foo !0, !bar !1
>>>>>>> > (the extra commas are required to disambiguate
from a named
>>>>>>> metadata on the next line)
>>>>>>>
>>>>>>> SGTM.
>>>>>>>
>>>>>>> > Benefits:
>>>>>>> > 1) Lets us reverse the DIGlobalVariable ->
GlobalVariable edge,
>>>>>>> which should hopefully clear the way for removing
the llvm.dbg.cu
>>>>>>> named metadata node.
>>>>>>>
>>>>>>> A little harder than it sounds (need to somehow
support a
>>>>>>> GlobalVariable that is RAUW'ed with a
ConstantInt), but I think this is
>>>>>>> important to do.
>>>>>>>
>>>>>>
>>>>>> How can a GlobalVariable be replaced with a
ConstantInt? Wouldn't
>>>>>> that just be an absolute address?
>>>>>>
>>>>>
>>>>> If the global variable has a known constant value & the
address is
>>>>> never needed, then we might optimize away the storage and
still want debug
>>>>> info describing the constant.
>>>>>
>>>>
>>>> I'm not sure under what conditions, if any, that currently
works - so
>>>> this may be more of a "nice to have" or "be good
to think about how any
>>>> future design wouldn't preclude fixing this gap" sort
of thing.
>>>>
>>>
>>> It looks like this is how debug info for static data members used
to
>>> look until r172591 landed back in 2013.
>>>
>>
>> Could you explain more what you mean by "<this> is how it
worked until" -
>> 'this' being how it used to work and how you're proposing
to make it work?
>> I'm not quite following.
>>
>
> Sorry, I meant that prior to r172591 we would represent a static data
> member with something that looked like what a DIGlobalVariable looks like
> today, but with a ConstantInt representing the initializer in the variable
> field (which is where the GlobalVariable is normally stored). The change
> moved the initializer to an associated "static member type" (yes,
it's not
> really a type), which as a side effect caused us to (as far as I can tell)
> no longer store ConstantInts in the variable field.
>
Actually this isn't accurate, we still produce a ConstantInt in the
variable field of DIGlobalVariable for constants in the global scope (e.g.
test/CodeGen/2010-08-10-DbgConstant.c in clang). I'll have to think more
about what the right representation is for that then.

Thanks,
-- 
-- 
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160509/7d0b9240/attachment.html>