[Openmp-commits] [PATCH] D134396: [OpenMP][libomptarget] New plugin infrastructure and new CUDA plugin

[Joseph Huber via Phabricator via Openmp-commits]

jhuber6 added inline comments.


================
Comment at: openmp/libomptarget/plugins-nextgen/common/PluginInterface/GlobalHandler.h:105
+  /// Map to store the ELF object files that have been loaded
+  std::unordered_map<int32_t, ELF64LEObjectFile> ELFObjectFiles;
+  std::mutex ELFObjectFilesMutex;
----------------
kevinsala wrote:
> jhuber6 wrote:
> > 
> Does `llvm::DenseMap` keep the references to the previously inserted elements valid when someone else is inserting a new element? I understood it invalidates them, but maybe I'm wrong. In case they are invalidated, we couldn't switch to `llvm::DenseMap` directly. I implemented this section thinking on multiple threads checking ELF object files, where some threads may be consulting an already inserted ELFObjectFile (without holding the map lock) while another potential thread may be inserting a new one (with the lock acquired).
You're right on the iterator invalidation. That guarantee is pat of what makes `std::unordered_map` slower than `llvm::DenseMap` in general. I'll leave it up to your discretion if you think that we need to be extra careful about invalidating iterators here. I was just guessing that this was part of the initialization code, which is generally thread safe since it should be called from the executable's constructors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134396