[PATCH] D25345: [ThinLTO] Handle empty summaries during internalization

[Teresa Johnson via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D25345#564860, @mehdi_amini wrote:

> The model I have in mind with the thin-link is that we operate on the combined index which represent all the files we're processing accurately. The linker provides us with the external references (among other informations).
>
> As I mentioned to you in person the other day, this model is broken by modules with empty summaries: some of the combined index transformations may be wrong (miscompile) because of that.
>
> We need to rework a bit the model for these modules...


Yes I was thinking about this some more after our conversation Wed. I think the right way to go is to do something like we do now with the HasSection flag which prevents importing any references - we can either add another bit or generalize that one - then set it for all GVs. That way they can be in the reference graph but not be imported.

But this change is needed for now to keep the current model working and to be able to commit https://reviews.llvm.org/D25359 which depends on this.



================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:554
+  // being able to rename local references). If we don't bail out here
+  // then we would get an assert below in the MustPreserveGV callback.
+  if (DefinedGlobals.empty())
----------------
mehdi_amini wrote:
> Can you get there with an empty module? (As a module that does not define any IR Global?)
Sure, but then we wouldn't call back into MustPreserveGV so it is fine to exit here early.


https://reviews.llvm.org/D25345