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

Joseph Huber via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Tue Sep 27 20:09:37 PDT 2022

jhuber6 added inline comments.

Comment at: openmp/libomptarget/plugins-nextgen/common/PluginInterface/GlobalHandler.h:41
+  // holding a private copy of the name as a std::string
+  std::string Name;
+  uint32_t Size;
kevinsala wrote:
> jhuber6 wrote:
> > This should be able to be a `StringRef` as well.
> Do you want to change it to `StringRef` for performance reasons (i.e., avoid the std::string's dynamic memory)?
> Using `StringRef` here could be dangerous since we have no control on the string memory lifetime. This can be dangerous if a developer constructs a string on the stack, creates a `GlobalTy` variable, and passes that string as the name (e.g., as we do for the `exec_mode`). Thus, we would be adding an extra restriction on `GlobalTy`, where the user must guarantee that the string name has a longer lifetime than the `GlobalTy` object.
> I would keep it as it is now, and if we see there is a performance penalty on using `std::string` here, we can change it.
This is a huge patch so I can't really get a good view of the usage, but I figured that this will always point to a global inside the `omp_offloading_entires` section, which will guarantee we always have access to this memory as it's just a string constant in the binary itself. Does the new plugin interface add new constants not contained in the entry list?



More information about the Openmp-commits mailing list