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

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 13 11:07:38 PDT 2016


> 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.


More information about the llvm-commits mailing list