llvm dev - Sep 2015 - RFC: DI: Stop preserving types from dead functions

Way back in r107027, we started preserving type information of local
variables of functions that are optimized away.  This seemed strange to
me so I dug into the history: apparently, this is so that ctfconvert can
find these types (so they can be exposed in dtrace).

E.g., this commit made it so that for this C code:

    static void foo(void) { struct X { int b; } v; }

we always get the type information for foo.v, even if foo() is optimized
away.

I came across this when trying to reverse the direction of the IR's
DICompileUnit/DISubprogram links.  r107027 effectively forces us to
hold onto subprogram definitions that describe deleted functions.  This
seems quite weird to me :/.

I'm talking to people internally and hoping to find that we just don't
care anymore.  Even if we do, perhaps shoving these types into
'retainedTypes:' in `DICompileUnit` (only if -gkeep-all-types or some
such) will solve the problem for the ctfconvert use case (without
burdening others).

While I sort that out... does anyone else rely on this?  How?  Why?

(The attach patch effectively reverts r107027.)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-DI-Stop-preserving-types-from-dead-functions.patch
Type: application/octet-stream
Size: 7655 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150901/51836b39/attachment.obj>

On Tue, Sep 1, 2015 at 3:59 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
> Way back in r107027, we started preserving type information of local
> variables of functions that are optimized away.  This seemed strange to
> me so I dug into the history: apparently, this is so that ctfconvert can
> find these types (so they can be exposed in dtrace).
>
> E.g., this commit made it so that for this C code:
>
>     static void foo(void) { struct X { int b; } v; }
>
> we always get the type information for foo.v, even if foo() is optimized
> away.
>
> I came across this when trying to reverse the direction of the IR's
> DICompileUnit/DISubprogram links.  r107027 effectively forces us to
> hold onto subprogram definitions that describe deleted functions.

Aside: there's a CR currently underway on llvm-dev about whether we should
(or should not) emit declarations for functions that have been optimized
away (neglecting local variables/types/etc, we currently emit a function
declaration for a function even if it gets inlined and optimized away -
it's unclear if that's the right call (given that we don't emit
declarations for functions that are never called, I'm not sure there's a
strong argument to be made to keep these, but I'm undecided)). So if
you're
interested in removing optimized-away subprogrcams, you might want to weigh
in on that thread.

For my money I'd apply a similar logic to those types: we don't emit any
number of types we know during IRGen are unused, so I'm not sure there's
a
good reason to keep them if we discover they're unused a bit later.

> This
> seems quite weird to me :/.
>
> I'm talking to people internally and hoping to find that we just
don't
> care anymore.  Even if we do, perhaps shoving these types into
> 'retainedTypes:' in `DICompileUnit` (only if -gkeep-all-types or
some
> such) will solve the problem for the ctfconvert use case (without
> burdening others).
>
> While I sort that out... does anyone else rely on this?  How?  Why?
>
> (The attach patch effectively reverts r107027.)
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150901/38333358/attachment.html>

> On 2015-Sep-01, at 16:07, David Blaikie <dblaikie at gmail.com>
wrote:
> 
> 
> 
> On Tue, Sep 1, 2015 at 3:59 PM, Duncan P. N. Exon Smith <dexonsmith at
apple.com> wrote:
> Way back in r107027, we started preserving type information of local
> variables of functions that are optimized away.  This seemed strange to
> me so I dug into the history: apparently, this is so that ctfconvert can
> find these types (so they can be exposed in dtrace).
> 
> E.g., this commit made it so that for this C code:
> 
>     static void foo(void) { struct X { int b; } v; }
> 
> we always get the type information for foo.v, even if foo() is optimized
> away.
> 
> I came across this when trying to reverse the direction of the IR's
> DICompileUnit/DISubprogram links.  r107027 effectively forces us to
> hold onto subprogram definitions that describe deleted functions. 
> 
> Aside: there's a CR currently underway on llvm-dev about whether we
should (or should not) emit declarations for functions that have been optimized
away (neglecting local variables/types/etc, we currently emit a function
declaration for a function even if it gets inlined and optimized away - it's
unclear if that's the right call (given that we don't emit declarations
for functions that are never called, I'm not sure there's a strong
argument to be made to keep these, but I'm undecided)). So if you're
interested in removing optimized-away subprogrcams, you might want to weigh in
on that thread.
I can't find the thread :(.  Can you give me the subject line?

I'm guessing it's something along the lines of deleting this block:
--
    if (!D && !includeMinimalInlineScopes())
      // Lazily construct the subprogram if we didn't see either concrete or
      // inlined versions during codegen. (except in -gmlt ^ where we want
      // to omit these entirely)
      D = getOrCreateSubprogramDIE(SP);
--
in DwarfCompileUnit::finishSubprogramDefinition()?  That was something
I was planning on bringing up next...
> For my money I'd apply a similar logic to those types: we don't
emit any number of types we know during IRGen are unused, so I'm not sure
there's a good reason to keep them if we discover they're unused a bit
later.
Yeah, it seems like the logic should be pretty similar.
>  
> This
> seems quite weird to me :/.
> 
> I'm talking to people internally and hoping to find that we just
don't
> care anymore.  Even if we do, perhaps shoving these types into
> 'retainedTypes:' in `DICompileUnit` (only if -gkeep-all-types or
some
> such) will solve the problem for the ctfconvert use case (without
> burdening others).
> 
> While I sort that out... does anyone else rely on this?  How?  Why?
> 
> (The attach patch effectively reverts r107027.)