[PATCH] D49362: [ThinLTO] Internalize read only globals

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Just a quick comment below since I'm traveling today and out tomorrow and won't have time for a better response until probably Monday. You may be right on the issue of using the index vs just using the read only bit, need to look more closely at your explanation.



================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:828
+    // If import is disabled we should drop read-only attribute
+    // from all summaries to prevent internalization.
+    for (auto &P : Index)
----------------
evgeny777 wrote:
> tejohnson wrote:
> > Why is this necessary? Since internalizeImmutableGVs is only invoked from importFunctions(), presumably it doesn't have any effect anyway when not importing?
> On -lto-O0 ThinLTO import is entirely disabled, so we have to prevent internalization of read-only stuff or we'll get link errors.
Right but since the internalization is done via internalizeImmutableGVs which is called from importFunctions, if importing is disabled under -lto-O0 wouldn't the internalization not be done either?


https://reviews.llvm.org/D49362