Hi! On Thu, 20 Mar 2014 17:50:13 +0100, Bernd Schmidt <[email protected]> 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
pgpsFtB9UCkKj.pgp
Description: PGP signature
