[gomp4] Add tables generation

Hi!

On Fri, 4 Apr 2014 11:30:49 +0200, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 03/21/2014 04:20 PM, Jakub Jelinek wrote:
> > On Fri, Mar 21, 2014 at 04:13:45PM +0100, Bernd Schmidt wrote:
> >> On 03/20/2014 07:56 PM, Jakub Jelinek wrote:
> >>> When we were discussing the design last year, my strong preference was that
> >>> either this lives in some other crt object that mkoffload/linker plugin adds
> >>> to link, or that it would be completely mkoffload synthetized.
> >>
> >> mkoffload is only concerned with generating target images. These
> >> fragments are for the host tables.
> >>
> >> How's this? It moves everything to ompbegin.o/ompend.o and only
> >> links in these files if we have produced at least one target offload
> >> image.
> >
> > I'd call the files crtompbegin.o/crtompend.o instead.

I'd go with crtoffload* (or similar).  ;-)


> > Also, supposedly if you've used section names without . in them, the linker
> > itself would provide the symbols automatically and you wouldn't actually
> > need begin/end, but just one object that would reference the linker created
> > symbols.  Just use say __gnu_offload_whatever__ or similar section names.
> 
> I've checked in the following which should address all this.

Is it a linker bug that I need to add something like the following?


    $ ld --version
    GNU ld (Sourcery CodeBench 2013.11-17) 2.23.52.20130912
    [...]


> --- libgcc/ompstuff.c	(revision 0)
> +++ libgcc/ompstuff.c	(working copy)

> +extern void __start___gnu_offload_funcs;
> +extern void __stop___gnu_offload_funcs;
> +extern void __start___gnu_offload_vars;
> +extern void __stop___gnu_offload_vars;
> +void *__OPENMP_TARGET__[] __attribute__ ((__visibility__ ("hidden"))) =
> +{
> +  &__start___gnu_offload_funcs, &__stop___gnu_offload_funcs,
> +  &__start___gnu_offload_vars, &__stop___gnu_offload_vars
> +};

    ../../../source/libgcc/ompstuff.c:49:3: warning: taking address of expression of type 'void'
       &__start___gnu_offload_funcs, &__stop___gnu_offload_funcs,
       ^
    ../../../source/libgcc/ompstuff.c:49:33: warning: taking address of expression of type 'void'
       &__start___gnu_offload_funcs, &__stop___gnu_offload_funcs,
                                     ^
    ../../../source/libgcc/ompstuff.c:50:3: warning: taking address of expression of type 'void'
       &__start___gnu_offload_vars, &__stop___gnu_offload_vars
       ^
    ../../../source/libgcc/ompstuff.c:50:32: warning: taking address of expression of type 'void'
       &__start___gnu_offload_vars, &__stop___gnu_offload_vars
                                    ^

s%void%char makes this go away.


Grüße,
 Thomas

On 04/05/2014 05:04 PM, Thomas Schwinge wrote:
> Is it a linker bug that I need to add something like the following?
>
> --- libgcc/ompstuff.c
> +++ libgcc/ompstuff.c
> @@ -40,6 +40,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>   #include "libgcc_tm.h"
>
>   #if defined(HAVE_GAS_HIDDEN) && defined(ENABLE_OFFLOADING)
> +# if 1
> +/* TODO: Without the following, will get missing symbols for __start_* and
> +   __stop_*.  Linker bug?  */
> +static void *_funcs[0] __attribute__ ((section ("__gnu_offload_funcs"))) = { };
> +static void *_vars[0] __attribute__ ((section ("__gnu_offload_vars"))) = { };
> +# endif

Things seemed to work over here, but now I'm not certain whether the 
__start_/__stop_ functionality is GNU ld specific? Maybe we should just 
go back to the previous version of this patch which didn't try to use this.


Bernd

On Sat, Apr 05, 2014 at 05:22:09PM +0200, Bernd Schmidt wrote:
> On 04/05/2014 05:04 PM, Thomas Schwinge wrote:
> >Is it a linker bug that I need to add something like the following?
> >
> >--- libgcc/ompstuff.c
> >+++ libgcc/ompstuff.c
> >@@ -40,6 +40,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
> >  #include "libgcc_tm.h"
> >
> >  #if defined(HAVE_GAS_HIDDEN) && defined(ENABLE_OFFLOADING)
> >+# if 1
> >+/* TODO: Without the following, will get missing symbols for __start_* and
> >+   __stop_*.  Linker bug?  */
> >+static void *_funcs[0] __attribute__ ((section ("__gnu_offload_funcs"))) = { };
> >+static void *_vars[0] __attribute__ ((section ("__gnu_offload_vars"))) = { };
> >+# endif
> 
> Things seemed to work over here, but now I'm not certain whether the
> __start_/__stop_ functionality is GNU ld specific? Maybe we should
> just go back to the previous version of this patch which didn't try
> to use this.

Somebody needs to try it with gold, I think it should support the same.
As for other linkers, don't we need linker plugin support anyway, which is
not available for other linkers?

	Jakub

On 05 Apr 17:22, Bernd Schmidt wrote:
> Things seemed to work over here, but now I'm not certain whether the
> __start_/__stop_ functionality is GNU ld specific? Maybe we should
> just go back to the previous version of this patch which didn't try
> to use this.
> 
> Bernd

This approach does not work with shared libraries.

The automatically inserted symbols have GLOBAL binding, therefore the
__start_/__stop_ from the executable overwrite the respective symbols in DSO.

Here is a simple example with 2 DSOs and one executable.  The function
GOMP_offload_register is called with the following pointers in HOST_TABLE:

1. (funcs 0x604880:0x604898, vars 0x604840:0x604880)
2. (funcs 0x604880:0x604898, vars 0x604840:0x604880)
3. (funcs 0x604880:0x604898, vars 0x604840:0x604880)

But with "manually" added start/stop and LOCAL binding everything works fine:

1. (funcs 0x7f286b425530:0x7f286b425540, vars 0x7f286b425540:0x7f286b425540)
2. (funcs 0x7f286b8624a0:0x7f286b8624b0, vars 0x7f286b8624b0:0x7f286b8624b0)
3. (funcs 0x604760:0x604778, vars 0x604780:0x6047c0)

  -- Ilya