Hi!
On Sat, 20 Feb 2016 13:54:20 +0300, Ilya Verbin <iver...@gmail.com> wrote:
> On Fri, Feb 19, 2016 at 15:53:08 +0100, Jakub Jelinek wrote:
> > On Wed, Feb 10, 2016 at 08:19:34PM +0300, Ilya Verbin wrote:
> > > This patch adds crtoffload{begin,end}.o to all -fopenmp programs, if they
> > > exist.
> > > I couldn't think of a better solution...
> > > Tested using the testcase from the previous mail, e.g.:
> > >
> > > $ gcc -DNUM=1 -c -fopenmp test.c -o obj1.o
> > > $ gcc -DNUM=2 -c -fopenmp test.c -o obj2.o
> > > $ gcc -DNUM=3 -c -fopenmp test.c -o obj3.o
> > > $ gcc -DNUM=4 -c -fopenmp test.c -o obj4.o -flto
> > > $ gcc -DNUM=5 -c -fopenmp test.c -o obj5.o
> > > $ gcc -DNUM=6 -c -fopenmp test.c -o obj6.o -flto
> > > $ gcc -DNUM=7 -c -fopenmp test.c -o obj7.o
> > > $ gcc-ar -cvq libtest.a obj3.o obj4.o obj5.o
> > > $ gcc -fopenmp main.c obj1.o obj2.o libtest.a obj6.o obj7.o
> > >
> > > And other combinations.
> Thomas, could you please test it using nvptx
It mostly ;-) works. With nvptx offloading enabled (which you don't
have, do you?), I'm seeing one test case regress:
[-PASS:-]{+FAIL:+}
libgomp.oacc-c/../libgomp.oacc-c-c++-common/parallel-dims-2.c
-DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 (test for errors, line 9)
[-PASS:-]{+FAIL:+}
libgomp.oacc-c/../libgomp.oacc-c-c++-common/parallel-dims-2.c
-DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 (test for errors, line 13)
PASS: libgomp.oacc-c/../libgomp.oacc-c-c++-common/parallel-dims-2.c
-DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 (test for excess errors)
[-PASS:-]{+FAIL:+}
libgomp.oacc-c/../libgomp.oacc-c-c++-common/parallel-dims-2.c
-DACC_DEVICE_TYPE_nvidia=1 -DACC_MEM_SHARED=0 execution test
(Same for C++.) That testcase, just recently added by Tom in r233237
"Handle -fdiagnostics-color in lto", specifies 'dg-additional-options
"-flto -fno-use-linker-plugin"'. Is that now an unsupported
combination/configuration? (I have not yet looked in detail, but it
appears as if the offloading compilers are no longer being run for
-fno-use-linker-plugin.)
> including the testcase with static
> libraries?
Works in my manual testing if I work around the following issue:
> --- a/gcc/config/gnu-user.h
> +++ b/gcc/config/gnu-user.h
> @@ -49,14 +49,16 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
> If not, see
> %{" NO_PIE_SPEC ":crtbegin.o%s}} \
> %{fvtable-verify=none:%s; \
> fvtable-verify=preinit:vtv_start_preinit.o%s; \
> - fvtable-verify=std:vtv_start.o%s}"
> + fvtable-verify=std:vtv_start.o%s} \
> + %{fopenacc|fopenmp:%:if-exists(crtoffloadbegin%O%s)}"
(..., and similar for others.) The if-exists spec function only works
for absolute paths (I have not researched, why?), so it won't locate the
files for relative -Bbuild-gcc/[...] prefixes, and linking will fail:
/tmp/ccGajPD4.crtoffloadtable.o:(.rodata+0x0): undefined reference to
`__offload_func_table'
/tmp/ccGajPD4.crtoffloadtable.o:(.rodata+0x8): undefined reference to
`__offload_funcs_end'
/tmp/ccGajPD4.crtoffloadtable.o:(.rodata+0x10): undefined reference to
`__offload_var_table'
/tmp/ccGajPD4.crtoffloadtable.o:(.rodata+0x18): undefined reference to
`__offload_vars_end'
If I use the absolute -B$PWD/build-gcc/[...], it works. (But there is no
requirement for -B prefixes to be absolute, as far as I know.) Why not
make it a hard error, though, if these files are missing? Can we use
something like (untested pseudo-patch):
+#ifdef ENABLE_OFFLOADING
+# define CRTOFFLOADBEGIN "%{fopenacc|fopenmp:%:crtoffloadbegin%O%s}"
+#else
+# define CRTOFFLOADBEGIN ""
+#endif
@@ -49,14 +49,16 @@ see the files COPYING3 and COPYING.RUNTIME
respectively. If not, see
%{" NO_PIE_SPEC ":crtbegin.o%s}} \
%{fvtable-verify=none:%s; \
fvtable-verify=preinit:vtv_start_preinit.o%s; \
- fvtable-verify=std:vtv_start.o%s}"
+ fvtable-verify=std:vtv_start.o%s} \
+ " CRTOFFLOADBEGIN ")}"
I have not verified your patch's logic in detail (arcane...) ;-) so just
two drive-by comments:
> #else
> #define GNU_USER_TARGET_STARTFILE_SPEC \
> "%{!shared: %{pg|p|profile:gcrt1.o%s;:crt1.o%s}} \
> crti.o%s %{static:crtbeginT.o%s;shared|pie:crtbeginS.o%s;:crtbegin.o%s} \
> %{fvtable-verify=none:%s; \
> fvtable-verify=preinit:vtv_start_preinit.o%s; \
> - fvtable-verify=std:vtv_start.o%s}"
> + fvtable-verify=std:vtv_start.o%s} \
> + %{fopenacc|fopenmp:%:if-exists(crtoffloadbegin%O%s)}"
> #endif
> #undef STARTFILE_SPEC
> #define STARTFILE_SPEC GNU_USER_TARGET_STARTFILE_SPEC
> @@ -73,13 +75,15 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
> If not, see
> fvtable-verify=preinit:vtv_end_preinit.o%s; \
> fvtable-verify=std:vtv_end.o%s} \
> %{shared:crtendS.o%s;: %{" PIE_SPEC ":crtendS.o%s} \
> - %{" NO_PIE_SPEC ":crtend.o%s}} crtn.o%s"
> + %{" NO_PIE_SPEC ":crtend.o%s}} crtn.o%s \
> + %{fopenacc|fopenmp:%:if-exists(crtoffloadend%O%s)}"
> #else
> #define GNU_USER_TARGET_ENDFILE_SPEC \
> "%{fvtable-verify=none:%s; \
> fvtable-verify=preinit:vtv_end_preinit.o%s; \
> fvtable-verify=std:vtv_end.o%s} \
> - %{shared|pie:crtendS.o%s;:crtend.o%s} crtn.o%s"
> + %{shared|pie:crtendS.o%s;:crtend.o%s} crtn.o%s \
> + %{fopenacc|fopenmp:%:if-exists(crtoffloadend%O%s)}"
> #endif
> #undef ENDFILE_SPEC
> #define ENDFILE_SPEC GNU_USER_TARGET_ENDFILE_SPEC
I guess we currently don't have to care about offloading configurations
not using the gnu-user.h file in which you modified the
STARTFILE_SPEC/ENDFILE_SPEC?
> --- a/lto-plugin/lto-plugin.c
> +++ b/lto-plugin/lto-plugin.c
> @@ -671,16 +681,37 @@ all_symbols_read_handler (void)
> + if (num_offload_files > 0)
> {
> + [...]
> + struct plugin_offload_file *ofld;
> + [...]
> + ofld = offload_files->next;
> + while (ofld)
> + {
> + fprintf (f, "%s\n", ofld->name);
> + ofld = ofld->next;
> + }
To the casual reader, skipping the first offload_files looks like a
off-by-one error, so I suggest you add a comment "Skip the dummy item at
the start of the list.", or similar.
Grüße
Thomas
