[Openmp-commits] [PATCH] D73657: [OPENMP] Load plugins from same directory as the libomptarget.so and quick fail mechanism for offloading plugins
Jon Chesterfield via Phabricator via Openmp-commits
openmp-commits at lists.llvm.org
Wed Jan 29 14:17:39 PST 2020
JonChesterfield added a comment.
@Ravi you're marked as needs changes but I can't see any comments
Comment at: openmp/libomptarget/src/rtl.cpp:67
+ // Load plugins from same directory as libomptarget.so
+ std::string full_plugin_name;
> Why do we want to do this? Also, does it mean that cmake configuration must be changed to put plugins in the same directory as libomptarget.so?
Setting LD_LIBRARY_PATH in order to work is nasty. I didn't realise that was the current status.
Loading relative to libomptarget is nicely position independent. Do you have a preferred location?
Comment at: openmp/libomptarget/src/rtl.cpp:72
+ DP("dlopen() failed: %s\n", dlerror());
+ char *libomptarget_dir_name = new char;
+ if (dlinfo(handle, RTLD_DI_ORIGIN, libomptarget_dir_name) == -1)
> Why need to use dynamic allocation here?
To keep it off the stack, but char seems fine as a local to me too.
Is there a cross platform equivalent to this in the llvm libraries? Seems a bit suspect that dlinfo doesn't take the size of the buffer it writes to.
Plus I thought PATH_MAX was 256 and Google suggests Linux sets it to 260 now, so a wrapper with associated ready made error handling would be nice.
Comment at: openmp/libomptarget/src/rtl.cpp:93
+ if (!found) // Not finding quick check files is a faster fail than dlopen
I'm not as sure about this. Probably faster, but is dlopen that slow when the file doesn't exist? Seems to run at terminal print speed when I'm debugging
CHANGES SINCE LAST ACTION
More information about the Openmp-commits