[PATCH 1/4] Remove build dependence on HSA run-time

[Thomas Schwinge]

Hi!

I'm raising here an issue with HSA libgomp plugin code changes from a
while ago.  While HSA is now no longer relevant for GCC master branch,
the same code has also been copied into the GCN libgomp plugin.

This is commit b8d89b03db5f212919e4571671ebb4f5f8b1e19d (r242749) "Remove
build dependence on HSA run-time":

On 2016-11-22T14:27:44+0100, Martin Jambor <mjambor@suse.cz> wrote:
> --- a/libgomp/plugin/configfrag.ac
> +++ b/libgomp/plugin/configfrag.ac

> @@ -195,8 +183,8 @@ if test x"$enable_offload_targets" != x; then
>               tgt_name=hsa
>               PLUGIN_HSA=$tgt
>               PLUGIN_HSA_CPPFLAGS=$HSA_RUNTIME_CPPFLAGS
> -             PLUGIN_HSA_LDFLAGS="$HSA_RUNTIME_LDFLAGS $HSA_KMT_LDFLAGS"
> -             PLUGIN_HSA_LIBS="-lhsa-runtime64 -lhsakmt"
> +             PLUGIN_HSA_LDFLAGS="$HSA_RUNTIME_LDFLAGS"
> +             PLUGIN_HSA_LIBS="-ldl"

So this switched from directly linking against 'libhsa-runtime64.so' to a
'libdl'-based runtime linking variant.

Previously, 'libhsa-runtime64.so' would've been found at run time via the
standard search paths.

> +if test "$HSA_RUNTIME_LIB" != ""; then
> +  HSA_RUNTIME_LIB="$HSA_RUNTIME_LIB/"
> +fi
> +
> +AC_DEFINE_UNQUOTED([HSA_RUNTIME_LIB], ["$HSA_RUNTIME_LIB"],
> +  [Define path to HSA runtime.])

That's new, to propagate '--with-hsa-runtime'/'--with-hsa-runtime-lib'
into the HSA plugin source code.

> --- a/libgomp/plugin/plugin-hsa.c
> +++ b/libgomp/plugin/plugin-hsa.c

> +static const char *hsa_runtime_lib;

>  static void
>  init_enviroment_variables (void)
>  {

> +  hsa_runtime_lib = secure_getenv ("HSA_RUNTIME_LIB");

Unless overridden via the 'HSA_RUNTIME_LIB' environment variable...

> +  if (hsa_runtime_lib == NULL)
> +    hsa_runtime_lib = HSA_RUNTIME_LIB "libhsa-runtime64.so";

... we now default to '[HSA_RUNTIME_LIB]/libhsa-runtime64.so' (note
'HSA_RUNTIME_LIB' prefix!)...

> +static bool
> +init_hsa_runtime_functions (void)
> +{
> +  void *handle = dlopen (hsa_runtime_lib, RTLD_LAZY);

..., which is then 'dlopen'ed here.

That means, contrary to before, the GCC configure-time
'--with-hsa-runtime' (by definition only valid for GCC configure/build as
well as build-tree testing) leaks into the installed HSA libgomp plugin.
That's a problem if your GCC build system (and build-tree testing)
requires '--with-hsa-runtime' to specify a non-standard location (not in
default search paths) but that location is not valid on your GCC
deployment system (but it has leaked into the HSA libgomp plugin),
meaning that (unless overridden via the 'HSA_RUNTIME_LIB' environment
variable) 'libhsa-runtime64.so' is now no longer found via the standard
search paths, because of the 'HSA_RUNTIME_LIB' prefix passed into
'dlopen'.

Per my understanding this cannot be intentional, so I suggest to restore
the previous behavior as per the attached "libgomp HSA/GCN plugins: don't
prepend the 'HSA_RUNTIME_LIB' path to 'libhsa-runtime64.so'".  OK to push
such changes?  I was tempted to push "as obvious", but maybe I fail to
see the rationale behind this change?

For avoidance of doubt, this change doesn't affect (build-tree) testsuite
usage, where we have:

    libgomp/testsuite/libgomp-test-support.exp.in:set hsa_runtime_lib "@HSA_RUNTIME_LIB@"

    libgomp/testsuite/lib/libgomp.exp:          append always_ld_library_path ":$hsa_runtime_lib"

And, another data point:

    gcc/config/gcn/gcn-run.c:#define HSA_RUNTIME_LIB "libhsa-runtime64.so.1"
    [...]
    gcc/config/gcn/gcn-run.c:  void *handle = dlopen (HSA_RUNTIME_LIB, RTLD_LAZY);

Here, 'libhsa-runtime64.so.1' is 'dlopen'ed without prefix, and thus
found via the standard search paths (as expected).


Grüße
 Thomas


-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-libgomp-HSA-GCN-plugins-don-t-prepend-the-HSA_RU.g10.patch
Type: text/x-diff
Size: 3038 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20210114/393d231b/attachment.bin>