[PATCH][RFC][Offloading] Fix PR68463

[Thomas Schwinge]

Hi!

On Sat, 20 Feb 2016 13:54:20 +0300, Ilya Verbin <iverbin@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