This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 3/4] Add libgomp plugin for Intel MIC
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Ilya Verbin <iverbin at gmail dot com>
- Cc: gcc-patches at gcc dot gnu dot org, Kirill Yukhin <kirill dot yukhin at gmail dot com>, Andrey Turetskiy <andrey dot turetskiy at gmail dot com>
- Date: Wed, 22 Oct 2014 11:22:05 +0200
- Subject: Re: [PATCH 3/4] Add libgomp plugin for Intel MIC
- Authentication-results: sourceware.org; auth=none
- References: <20141021171323 dot GA47586 at msticlxl57 dot ims dot intel dot com> <20141021172413 dot GD47586 at msticlxl57 dot ims dot intel dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Tue, Oct 21, 2014 at 09:24:13PM +0400, Ilya Verbin wrote:
> +/* Add path specified in LD_LIBRARY_PATH to MIC_LD_LIBRARY_PATH, which is
> + required by liboffloadmic. */
> +__attribute__((constructor))
> +static void
> +set_mic_lib_path (void)
> +{
> + const char *ld_lib_path = getenv (LD_LIBRARY_PATH_ENV);
> + const char *mic_lib_path = getenv (MIC_LD_LIBRARY_PATH_ENV);
> + char *mic_lib_path_new;
> +
> + if (!ld_lib_path)
> + return;
> +
> + mic_lib_path_new = (char *) malloc ((mic_lib_path ? strlen (mic_lib_path) : 0)
> + + strlen (ld_lib_path) + 2);
malloc can fail, SIGSEGV in response to that is not desirable.
Can't you fallback to alloca, or use just alloca, or use alloca
with malloc fallback?
> +
> + if (!mic_lib_path)
> + strcpy (mic_lib_path_new, ld_lib_path);
> + else
> + sprintf (mic_lib_path_new, "%s:%s", mic_lib_path, ld_lib_path);
> + setenv (MIC_LD_LIBRARY_PATH_ENV, mic_lib_path_new, 1);
> + free (mic_lib_path_new);
> +extern "C" void
> +GOMP_OFFLOAD_register_image (void *host_table, void *target_image)
> +{
> + TRACE ("(host_table = %p, target_image = %p)", host_table, target_image);
> +
> + if (num_libraries >= 1000)
> + {
> + fprintf (stderr,
> + "%s: The number of loaded shared libraries is over 1000!\n",
> + __FILE__);
> + exit (1);
Where does this artificial limit come from? Using libNNN.so library names?
Can't you use lib%d.so instead?
Also, seeing register_image, shouldn't there be
GOMP_OFFLOAD_unregister_image which would be invoked when the library
containing MIC offloading regions is dlclosed?
One could use __cxa_atexit or similar for that, something that is given
&__dso_handle. Or is no cleanup necessary? At least unregistering it
from translation tables, because the same addresses might be reused by a
different shared library?
With dlopen/dlclose in mind, 1000 might be easily reached, consider 10000
times dlopening/dlclosing (perhaps over longer time, by long running daemon)
a shared library containg #pragma omp target region.
> +static int first_init = 1;
> +
> +/* Load offload_target_main on target. */
> +extern "C" void
> +GOMP_OFFLOAD_init_device (int device)
> +{
> + TRACE ("");
> + pthread_mutex_lock (&mutex);
> + if (first_init)
> + {
> + __offload_register_image (&main_target_image);
> + first_init = 0;
> + }
> + pthread_mutex_unlock (&mutex);
pthread_once instead?
Jakub