[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