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

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

A few suggestions for beefing up the comments



================
Comment at: llvm/lib/IR/ModuleSummaryIndex.cpp:200
   auto HasRefsPreventingImport = [this](const GlobalVarSummary *GVS) {
-    return !isReadOnly(GVS) && GVS->refs().size();
+    return !isReadOnly(GVS) && !isWriteOnly(GVS) && GVS->refs().size();
   };
----------------
Please add comment about why refs on write only vars must be ignored for correctness, and that we must import their defs.


================
Comment at: llvm/lib/Transforms/Utils/FunctionImportUtils.cpp:261
+          // to avoid promotion of referenced objects which aren't accessed
+          // anyway.
+          V->setInitializer(Constant::getNullValue(V->getValueType()));
----------------
Please add a note that we have to do this as the variable importing code does not mark these references as exported.


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

https://reviews.llvm.org/D70006