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

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D49362#1292750, @evgeny777 wrote:

> Sorry I have to suggest small amendment to this patch before committing after looking `test/ThinLTO/X86/distributed_import.ll`.
>  It looks like distributed import is using per-module indexes, so internalization of RO vars will not work correctly. To overcome this
>  I suggest adding a check for propagateConstants to have been run before marking GV for internalization
>
> See https://reviews.llvm.org/D54306
>
> This change causes modifications to `FunctionImport/funcimport.ll` to be reverted. The check for `withGlobalValueDeadStripping` is a bit counter intuitive, so suggestions welcome.


Why won't it work for distributed builds? The read only gvar flag is being serialized out and back in, so shouldn't it show up in the distributed indexes? If not, the problem should be fixed there so it can work with distributed builds, rather than skipping it (otherwise e.g. here at Google we won't be able to take advantage of this).


https://reviews.llvm.org/D49362