[PATCH] D95943: [ThinLTO] Fix import of a global variable to modules contain its weak def

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Thanks. AFAICT this is a reasonable fix. Suggestion below for a better comment and FIXME to add.



================
Comment at: llvm/lib/Transforms/IPO/FunctionImport.cpp:288
+    if (GVS != DefinedGVSummaries.end()) {
+      if (VI.getSummaryList().size() > 1 &&
+          GlobalValue::isInterposableLinkage(GVS->second->linkage())) {
----------------
Can you add a comment here about how this is required for correctness?
Also, add a FIXME that we can refine this further if we pass down the prevailing symbol information, and only need to allow imports if this is not the prevailing copy, and further if the prevailing copy is marked read only?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95943