[PATCH] D16440: [ThinLTO] Link in only necessary DICompileUnit operands

[Teresa Johnson via llvm-commits]

tejohnson added a comment.

In http://reviews.llvm.org/D16440#338695, @dblaikie wrote:

> It's a little unclear from your description - are you importing /any/ types as definitions? Or are all imported types being imported as declarations? (I think the latter would be good/correct/minimal/simpler)


I think so. At least for composite types. Are there any cases, other than from the retainedTypes list on the compile unit, where a DICompositeType node is referenced by its address rather than by UUID? I haven't seen any, which means that we would not import any while mapping in metadata on imported functions/instructions. In that case they are only imported as declarations with this patch, since they are only imported by the new linkImportedCompileUnit routine.

Note that there are DIDerivedTypes that are referenced from imported functions (e.g. via the subprogram type array) and therefore mapped in when the functions and instructions are imported and mapped. Is there a distinction between definition and declaration for these types?


================
Comment at: test/Transforms/FunctionImport/funcimport_debug.ll:18
@@ +17,3 @@
+
+; Confirm that the enums didn't get pulled in to the imported compile unit
+; as it isn't needed here, but ensuring that retainedTypes immediately follows
----------------
dblaikie wrote:
> It might be clearer to match the retained types list (assuming all the types go in there, which I think they do - at least the ones using mangled names, etc) and then match all  the types referenced from there, then you wouldn't need the CHECK-NOTs which are a bit brittle here anyway (if types are emitted in a different order, the CHECK-NOTs wouldn't catch them - they'd only catch the case where the type is emitted between the two surrounding CHECK'd types)
> 
> Just a thought. (& could check that things like vtableHolder, etc, aren't emitted in the declarations you're interested in)
Good idea. I already found and fixed some issues where my existing CHECK-NOTs were not behaving as expected. Will do that.


http://reviews.llvm.org/D16440