Page MenuHomePhabricator

[OpenMP] Load plugins from same directory as the libomptarget.so
Needs ReviewPublic

Authored by saiislam on Sep 9 2020, 1:15 PM.

Details

Summary

Introduce a fall back mechanism to load plugins from the same
directory as libomptarget.so if library does not exist or cannnot
be found. It avoids the need to set LD_LIBRARY_PATH to find
plugins.

Diff Detail

Event Timeline

saiislam created this revision.Sep 9 2020, 1:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 9 2020, 1:15 PM
saiislam requested review of this revision.Sep 9 2020, 1:15 PM

First of two patches after splitting D73657

openmp/libomptarget/src/rtl.cpp
111–127

Do we need to repeat this everytime in the for loop

jdoerfert added inline comments.Sep 9 2020, 4:57 PM
openmp/libomptarget/src/rtl.cpp
99

Not needed.

111–127

We do not. If needed, try to find the path of libomptarget once. Please put that code in a helper method.

113
126–127
saiislam updated this revision to Diff 291033.Sep 10 2020, 11:07 AM

Moved the code to load plugins to a helper function and before loop on plugins.

See comments below. High-level: Why don't we say FullPluginName = Name, then try to load, if that fails, get the CWD and prepend it to FullPluginName before trying again. So LoadLibFromCWD will become bool getOrCreateCWD(std::string &CachedCWD) { if (CachedCWD) return true; ... } and a lot of the logic should be simpler. Also consider moving the actual loading part DP("Loading library '%s'...\n", Name); void *dynlib_handle = dlopen(Name, RTLD_NOW); into a helper so you can just call it twice instead of duplicating logic there again.

openmp/libomptarget/src/rtl.cpp
65

Why char** ? I mean, you fiddle with a char** here and below you handle both a char * and a std::string. Just go with the string.

123

Now there are two problems.

  1. You should not eagerly look into the CWD but only if we need to.
  2. If we need to and it fails we should emit a proper error message.