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

[Katya Romanova via Phabricator via llvm-commits]

kromanova added a comment.

Instead of storing pointers in the vector as you suggested, I think it will be better to store iterators. See the code snippet below.
This approach better fits C++ style. For example, this will guarantee that if operator -> for type pointer to MapEntryTy will be overloaded, it will not affect the logic of the code.
What do you think? If you agree, I will submit a new revision with these changes.

  // Include the hash for every module we import functions from. The set of
  // imported symbols for each module may affect code generation and is
  // sensitive to link order, so include that as well.
  using ImportMapIteratorTy = FunctionImporter::ImportMapTy::const_iterator;
  std::vector<ImportMapIteratorTy> ImportModulesVector;
  ImportModulesVector.reserve(ImportList.size());
  
  for (ImportMapIteratorTy it = ImportList.begin(); it != ImportList.end();
       ++it) {
    ImportModulesVector.push_back(it);
  }
  llvm::sort(
      ImportModulesVector,
      [](const ImportMapIteratorTy &Lhs, const ImportMapIteratorTy &Rhs) -> bool {
        return Lhs->getKey() < Rhs->getKey();
      });
  for (const ImportMapIteratorTy &EntryIt : ImportModulesVector) {
    auto ModHash = Index.getModuleHash(EntryIt->first());
    Hasher.update(ArrayRef<uint8_t>((uint8_t *)&ModHash[0], sizeof(ModHash)));
  
    AddUint64(EntryIt->second.size());
    for (auto &Fn : EntryIt->second)
      AddUint64(Fn);
  }


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

https://reviews.llvm.org/D79772