[PATCH] D70006: [ThinLTO] Fix bug when importing writeonly variables

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Ah, ok, I wasn't thinking about the part where the flag propagation was changed. I'm a little concerned that due to that change we are now required to import the variable as a definition. I guess there is no current way to disable importing of references while leaving on importing of functions (e.g. through some kind of internal option for debugging), but since such a mechanism might be useful at some point for debugging, my concern is that it will cause failures. Please add some very clear comments to the code where you are checking the write only flag as to why and what is going on. I.e. why needed for correctness and how we can ignore its references (because it will eventually be made zeroinit). Also, it would be good to add an assert somewhere to ensure that we don't export a reference but not a definition for a write only variable, to guard against any regression if someone does add a debugging mechanism to disable variable importing (which would need to affect the flag propagation). Maybe down at the bottom of ComputeCrossModuleImport where we prune the ExportLists.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70006/new/

https://reviews.llvm.org/D70006