[PATCH] D27775: [ThinLTO] Import composite types as declarations

[Adrian Prantl via llvm-commits]

> On Jan 3, 2017, at 3:31 PM, Teresa Johnson via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> tejohnson added inline comments.
> 
> 
> ================
> Comment at: llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp:750
> +        TemplateParams = getMDOrNull(Record[14]);
> +      }
>       DICompositeType *CT = nullptr;
> ----------------
> tejohnson wrote:
>> aprantl wrote:
>>> Sorry for bringing this up so late, but I just realized that this is only legal if the `Identifier` field is nonempty. The ODR (or an equivalent rule in the underlying source language) only applies to types with an identifier field. In C, for example, it is legal for multiple translation units to contain definitions of types with the same name, but a different layout. Using just the name it is impossible for the debugger to find the correct definition of a forward-declared type in the other translation units.
>> Ok, I can make the decl optimization conditional on the Identifier being non-null. I assume that is not the common case anyway - in which case that change shouldn't hurt the overall impact much?
> r290915
> 
> On Chromium, the fix increases the aggregate -g2 object file sizes with ThinLTO by almost 4%, but it just means a 65% reduction over the object files before this patch, instead of the original 66% reduction.

Given the size of Chromium this doesn't sound too bad for now. Thanks!

-- adrian

> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D27775
> 
> 
>