Hi!

On Thu, 20 Mar 2014 17:50:13 +0100, Bernd Schmidt <ber...@codesourcery.com> 
wrote:
> This is based on Michael Zolotukhin's patch 2/3 from a while ago. It 
> adds functionality to build function/variable tables that will allow 
> libgomp to look up offload target code based on the address of the 
> corresponding host function. There are two alternatives, one based on 
> named sections, and one based on a target hook when named sections are 
> unavailable (as on ptx).
> 
> Committed on gomp-4_0-branch.

I see regressions in the libgomp testsuite for configurations where
offloading is not enabled:

    spawn [...]/build/gcc/xgcc -B[...]/build/gcc/ 
[...]/source/libgomp/testsuite/libgomp.c/for-3.c 
-B[...]/build/x86_64-unknown-linux-gnu/./libgomp/ 
-B[...]/build/x86_64-unknown-linux-gnu/./libgomp/.libs 
-I[...]/build/x86_64-unknown-linux-gnu/./libgomp 
-I[...]/source/libgomp/testsuite/.. -fmessage-length=0 
-fno-diagnostics-show-caret -fdiagnostics-color=never -fopenmp -std=gnu99 
-fopenmp -L[...]/build/x86_64-unknown-linux-gnu/./libgomp/.libs -lm -o 
./for-3.exe
    /tmp/ccGnT0ei.o: In function `main':
    for-3.c:(.text+0x21032): undefined reference to `__OPENMP_TARGET__'
    collect2: error: ld returned 1 exit status

I suppose that's because even if...

> --- gcc/configure.ac  (revision 208715)
> +++ gcc/configure.ac  (working copy)
> @@ -887,6 +887,10 @@ AC_SUBST(enable_accelerator)
>  offload_targets=`echo $offload_targets | sed -e 's#,#:#'`
>  AC_DEFINE_UNQUOTED(OFFLOAD_TARGETS, "$offload_targets",
>   [Define to hold the list of target names suitable for offloading.])
> +if test x$offload_targets != x; then
> +  AC_DEFINE(ENABLE_OFFLOADING, 1,
> +    [Define this to enable support for offloading.])
> +fi

... offloading is not enabled, this...

> --- gcc/omp-low.c     (revision 208706)
> +++ gcc/omp-low.c     (working copy)
> @@ -8671,19 +8672,22 @@ expand_omp_target (struct omp_region *re
>      }
>  
>    gimple g;
> -  /* FIXME: This will be address of
> -     extern char __OPENMP_TARGET__[] __attribute__((visibility ("hidden")))
> -     symbol, as soon as the linker plugin is able to create it for us.  */
> -  tree openmp_target = build_zero_cst (ptr_type_node);
> +  tree openmp_target
> +    = build_decl (UNKNOWN_LOCATION, VAR_DECL,
> +               get_identifier ("__OPENMP_TARGET__"), ptr_type_node);
> +  TREE_PUBLIC (openmp_target) = 1;
> +  DECL_EXTERNAL (openmp_target) = 1;
>    if (kind == GF_OMP_TARGET_KIND_REGION)
>      {
>        tree fnaddr = build_fold_addr_expr (child_fn);
> -      g = gimple_build_call (builtin_decl_explicit (start_ix), 7,
> -                          device, fnaddr, openmp_target, t1, t2, t3, t4);
> +      g = gimple_build_call (builtin_decl_explicit (start_ix), 7, device,
> +                          fnaddr, build_fold_addr_expr (openmp_target),
> +                          t1, t2, t3, t4);
>      }
>    else
> -    g = gimple_build_call (builtin_decl_explicit (start_ix), 6,
> -                        device, openmp_target, t1, t2, t3, t4);
> +    g = gimple_build_call (builtin_decl_explicit (start_ix), 6, device,
> +                        build_fold_addr_expr (openmp_target),
> +                        t1, t2, t3, t4);

... will now cause a reference to __OPENMP_TARGET__, but...

> --- libgcc/crtstuff.c (revision 208706)
> +++ libgcc/crtstuff.c (working copy)
> @@ -311,6 +311,15 @@ register_tm_clones (void)
>  }
>  #endif /* USE_TM_CLONE_REGISTRY */
>  
> +#if defined(HAVE_GAS_HIDDEN) && defined(ENABLE_OFFLOADING)
> +void *_omp_func_table[0]
> +  __attribute__ ((__used__, visibility ("protected"),
> +               section (".offload_func_table_section"))) = { };
> +void *_omp_var_table[0]
> +  __attribute__ ((__used__, visibility ("protected"),
> +               section (".offload_var_table_section"))) = { };
> +#endif
> +
>  #if defined(INIT_SECTION_ASM_OP) || defined(INIT_ARRAY_SECTION_ASM_OP)
>  
>  #ifdef OBJECT_FORMAT_ELF
> @@ -752,6 +761,23 @@ __do_global_ctors (void)
>  #error "What are you doing with crtstuff.c, then?"
>  #endif
>  
> +#if defined(HAVE_GAS_HIDDEN) && defined(ENABLE_OFFLOADING)
> +void *_omp_funcs_end[0]
> +  __attribute__ ((__used__, visibility ("protected"),
> +               section (".offload_func_table_section"))) = { };
> +void *_omp_vars_end[0]
> +  __attribute__ ((__used__, visibility ("protected"),
> +               section (".offload_var_table_section"))) = { };
> +extern void *_omp_func_table[];
> +extern void *_omp_var_table[];
> +void *__OPENMP_TARGET__[] __attribute__ ((__visibility__ ("protected"))) =
> +{
> +  &_omp_func_table, &_omp_funcs_end,
> +  &_omp_var_table, &_omp_vars_end
> +};
> +#endif

... __OPENMP_TARGET__ is not being defined here for the
!ENABLE_OFFLOADING case.  In
<http://news.gmane.org/find-root.php?message_id=%3C20130905082455.GH23437%40tucnak.redhat.com%3E>,
Jakub had suggested this to be a weak symbol, so we'd get NULL in this
case, which would be what's needed here, I think?


Also, I'd suggest to rename __OPENMP_TARGET__ (and similar ones) to
__GNU_OFFLOAD__ (or similar).  As we're using this offloading stuff for
both OpenACC and OpenMP target, it makes sense to me to use a generic
name; we still have the chance to do so now while this stuff is not yet
in trunk.


Grüße,
 Thomas

Attachment: pgpsFtB9UCkKj.pgp
Description: PGP signature

Reply via email to