[PATCH] D79772: Nondeterminism of iterators causes false ThinLTO cache misses

[Katya Romanova via Phabricator via llvm-commits]

kromanova added inline comments.


================
Comment at: llvm/lib/LTO/LTO.cpp:168
+  for (StringRef ModId : ImportModulesVector) {
+    auto EntryIt = ImportList.find(ModId);
+    assert(EntryIt != ImportList.end() &&
----------------
mehdi_amini wrote:
> What about storing in the vector a reference to the entry in the first place and avoid the extra lookup here?
> 
> ```
>  using MapEntryTy = FunctionImporter::ImportMapTy::MapEntryTy;
>  std::vector<MapEntryTy *> ImportModulesVector;
>  ImportModulesVector.reserve(ImportList.size());
>  for (auto &Entry : ImportList) ImportModulesVector.push_back(&Entry);
>  llvm::sort(ImportModulesVector, [] (const MapEntryTy *Lhs, const MapEntryTy *Rhs) {
>     return Lhs < Rhs;
>  });
>  for (MapEntryTy *Entry : ImportModulesVector) {
>    ...
> ```
Good catch! I had to make a few changes to the code that you are suggesting. The rest of the code stayed the same. I will upload the updated patch to phabricator for one more round of reviews.

(1) I added const qualifier for Entry (auto &Entry : ImportList) -> (const auto &Entry : ImportList), otherwise this code doesn't compile.
(2) Same for (MapEntryTy *Entry : ImportModulesVector)-> (const MapEntryTy *Entry : ImportModulesVector)
(3) I assume that you didn't mean the compare the pointers here?
llvm::sort(ImportModulesVector, [] (const MapEntryTy *Lhs, const MapEntryTy *Rhs) {
        return Lhs < Rhs;

I changed it to 
llvm::sort(ImportModulesVector,
             [](const MapEntryTy *Lhs, const MapEntryTy *Rhs) {
               return Lhs->getKey() < Rhs->getKey();
             });
  


  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79772