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 <mjam...@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
>From 936e7ee10349a6be2bd0a6a2198f70239a8e1ec1 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tho...@codesourcery.com>
Date: Thu, 25 Jun 2020 11:59:42 +0200
Subject: [PATCH] libgomp HSA/GCN plugins: don't prepend the 'HSA_RUNTIME_LIB'
path to 'libhsa-runtime64.so'
For unknown reasons, this had gotten added for the libgomp HSA plugin in commit
b8d89b03db5f212919e4571671ebb4f5f8b1e19d (r242749) "Remove build dependence on
HSA run-time", and later propagated into the GCN plugin.
libgomp/
* plugin/plugin-hsa.c (init_enviroment_variables): Don't prepend
the 'HSA_RUNTIME_LIB' path to 'libhsa-runtime64.so'.
* plugin/plugin-gcn.c (init_environment_variables): Likewise.
* plugin/configfrag.ac (HSA_RUNTIME_LIB): Clean up.
* configure: Regenerate.
---
libgomp/configure | 10 ----------
libgomp/plugin/configfrag.ac | 7 -------
libgomp/plugin/plugin-gcn.c | 2 +-
libgomp/plugin/plugin-hsa.c | 2 +-
4 files changed, 2 insertions(+), 19 deletions(-)
diff --git a/libgomp/configure b/libgomp/configure
index d8d98f182d4..9765a9068fe 100755
--- a/libgomp/configure
+++ b/libgomp/configure
@@ -15483,16 +15483,6 @@ cat >>confdefs.h <<_ACEOF
_ACEOF
-if test "$HSA_RUNTIME_LIB" != ""; then
- HSA_RUNTIME_LIB="$HSA_RUNTIME_LIB/"
-fi
-
-
-cat >>confdefs.h <<_ACEOF
-#define HSA_RUNTIME_LIB "$HSA_RUNTIME_LIB"
-_ACEOF
-
-
# Check for functions needed.
for ac_func in getloadavg clock_gettime strtoull
diff --git a/libgomp/plugin/configfrag.ac b/libgomp/plugin/configfrag.ac
index fc91702a434..69a3cf4aeaf 100644
--- a/libgomp/plugin/configfrag.ac
+++ b/libgomp/plugin/configfrag.ac
@@ -310,10 +310,3 @@ AC_DEFINE_UNQUOTED([PLUGIN_HSA], [$PLUGIN_HSA],
AM_CONDITIONAL([PLUGIN_GCN], [test $PLUGIN_GCN = 1])
AC_DEFINE_UNQUOTED([PLUGIN_GCN], [$PLUGIN_GCN],
[Define to 1 if the GCN plugin is built, 0 if not.])
-
-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.])
diff --git a/libgomp/plugin/plugin-gcn.c b/libgomp/plugin/plugin-gcn.c
index 4c6a4c03b6e..d919de191fc 100644
--- a/libgomp/plugin/plugin-gcn.c
+++ b/libgomp/plugin/plugin-gcn.c
@@ -1074,7 +1074,7 @@ init_environment_variables (void)
hsa_runtime_lib = secure_getenv ("HSA_RUNTIME_LIB");
if (hsa_runtime_lib == NULL)
- hsa_runtime_lib = HSA_RUNTIME_LIB "libhsa-runtime64.so";
+ hsa_runtime_lib = "libhsa-runtime64.so";
support_cpu_devices = secure_getenv ("GCN_SUPPORT_CPU_DEVICES");
diff --git a/libgomp/plugin/plugin-hsa.c b/libgomp/plugin/plugin-hsa.c
index abd3bc64163..41951c464ef 100644
--- a/libgomp/plugin/plugin-hsa.c
+++ b/libgomp/plugin/plugin-hsa.c
@@ -254,7 +254,7 @@ init_enviroment_variables (void)
hsa_runtime_lib = secure_getenv ("HSA_RUNTIME_LIB");
if (hsa_runtime_lib == NULL)
- hsa_runtime_lib = HSA_RUNTIME_LIB "libhsa-runtime64.so";
+ hsa_runtime_lib = "libhsa-runtime64.so";
support_cpu_devices = secure_getenv ("HSA_SUPPORT_CPU_DEVICES");
}
--
2.17.1
