[PATCH] D70128: [ThinLTO] Add correctness check for RO/WO variable import

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

Thanks for doing this! A minor suggestion and a spelling correction on the variable below, but otherwise lgtm. Please commit the ExportList type change as a separate prior NFC commit and the new assertion checking after as a distinct commit.



================
Comment at: llvm/lib/LTO/LTO.cpp:423
+    thinLTOInternalizeAndPromoteGUID(I.second.SummaryList,
+                                     ValueInfo(Index.haveGVs(), &I), isExported,
                                      isPrevailing);
----------------
Just do Index.getValueInfo(I)


================
Comment at: llvm/lib/Transforms/IPO/FunctionImport.cpp:632
+
+  DenseSet<GlobalValue::GUID> FlattendedImports;
+
----------------
s/Flattended/Flattened/


================
Comment at: llvm/lib/Transforms/IPO/FunctionImport.cpp:636
+    for (auto &ExportPerModule : ImportPerModule.second)
+      for (auto GUID : ExportPerModule.second)
+        FlattendedImports.insert(GUID);
----------------
I think this loop could be simplified to just:
   FlattenedImports.insert(ExportPerModule.second.begin(), ExportPerModule.second.end());


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

https://reviews.llvm.org/D70128