[PATCH] D19034: PR27284 - Reverse the ownership between DICompileUnit and DISubprogram

[David Blaikie via llvm-commits]

On Wed, Apr 13, 2016 at 11:07 AM, Duncan P. N. Exon Smith via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

>
> > On 2016-Apr-13, at 10:53, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > dblaikie accepted this revision.
> > dblaikie added a comment.
> > This revision is now accepted and ready to land.
> >
> > Generally looks good - bunch of cleanup sort of stuff & a few casual
> questions, none of which I think would block this being committed.
> >
> >
> >
> > ================
> > Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:1101
> > @@ -1100,3 +1100,3 @@
> >   Record.push_back(VE.getMetadataOrNullID(N->getRetainedTypes().get()));
> > -  Record.push_back(VE.getMetadataOrNullID(N->getSubprograms().get()));
> > +  Record.push_back(/* subprograms */ 0);
> >
>  Record.push_back(VE.getMetadataOrNullID(N->getGlobalVariables().get()));
> > ----------------
> > Not worth changing the record layout to have one fewer records?
>
> Adrian chatted about this with me before sending out the patch.  I was
> skeptical at first, but he convinced me it's okay as is.  Here's my
> current thinking:
>
>   - It's as easy to change later as to change now.
>
>   - There are relatively few DICompileUnit records, so this has a
>     negligible effect on the size of the bitcode on disk.
>
>   - Changing the record layout complicates the logic on the reader
>     side, which is a non-negligible maintenance burden.
>

Yep, figured something along those lines but didn't hurt to ask/have it
written out explicitly. If we end up doing this to other lists (I could
imagine doing this to the global variables list too, for the same reasons,
right?) then we can do a batch cleanup at some point.

- Dave


> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160413/10e33396/attachment.html>