[Bug middle-end/109128] New: [Offload][OpenMP][OpenACC] Static linking with unused offload function will lead to mismatch number of offload fn/symbols

[Bug middle-end/109128] New: [Offload][OpenMP][OpenACC] Static linking with unused offload function will lead to mismatch number of offload fn/symbols

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109128

            Bug ID: 109128
           Summary: [Offload][OpenMP][OpenACC] Static linking with unused
                    offload function will lead to mismatch number of
                    offload fn/symbols
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: openacc, openmp, wrong-code
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: burnus at gcc dot gnu.org
                CC: jakub at gcc dot gnu.org
  Target Milestone: ---

gfortran -fopenmp one.f90
ar rcv libone.a one.o
ranlib libone.a
gfortran -fopenmp two.f90 -L. -lone

With GCN, it prints:
    libgomp: Cannot map target functions or variables (expected 0, have 2)

The issue seems to be related to the linker removing the the symbol from
__OFFLOAD_TABLE__ (i.e. from the .gnu.offload_funcs sections, which has
first/last the__offload_func_table and __offload_funcs_end symbols, whose
pointer is put into the __OFFLOAD_TABLE__).

But the removed symbol is still visible to the offload compiler, i.e. it ends
up in the GOMP_register_var call as generated by
gcc/config/{gcn,nvptx}/mkoffload.cc


It is not quite clear which permutations work or cause problems.
It seems as if the common block (both files have then '.comm' sections) is
important. Whether 'libone.a' also fails or only '-L. -lone' seems to be
inconsistent (some minor details). Additionally, having a libgomp library call
also in the main program is also required.


==> one.f90 <==
module m
  implicit none
  integer :: my_var
  common /my_common/ my_var
contains
  integer function unused_func(n) result(res)
     integer :: n
     !$omp target map(from:res) firstprivate(n)
       res = 5*n
     !$omp end target
  end
end module m

==> two.f90 <==
use m
implicit none
integer :: A
!$omp target enter data map(A)
end

[Bug middle-end/109128] [Offload][OpenMP][OpenACC] Static linking with unused offload function will lead to mismatch number of offload fn/symbols

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109128

Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2023-03-14
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
                 CC|                            |tschwinge at gcc dot gnu.org

--- Comment #1 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
See also
<https://inbox.sourceware.org/gcc-patches/711d1dba-0574-c7e6-0639-e49c105ee18b@codesourcery.com>
"Allow the accelerator to have more offloaded functions than the host".

[Bug middle-end/109128] [Offload][OpenMP][OpenACC] Static linking with unused offload function will lead to mismatch number of offload fn/symbols

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109128

--- Comment #2 from Tobias Burnus <burnus at gcc dot gnu.org> ---
(In reply to Thomas Schwinge from comment #1)
> See also "Allow the accelerator to have more offloaded functions than the host".

Which was:

-  if (num_target_entries != num_funcs + num_vars)
+  if (num_target_entries < num_funcs + num_vars)

I think this approach only works if functions are missing "at the end".
The code does:

gomp_load_image_to_device (..., const void *host_table,
                                const void *target_data, ...)
...
  void **host_func_table = ((void ***) host_table)[0];
  void **host_funcs_end  = ((void ***) host_table)[1];
  int num_funcs = host_funcs_end - host_func_table;
...
  num_target_entries
    = devicep->load_image_func (devicep->target_id, version,
                                target_data, &target_table,
...
      k->host_start = (uintptr_t) host_func_table[i];
...
      k->tgt_offset = target_table[i].start;

Thus, it strictly depends on the order. That is: If some function "in the
middle" is not linked in / part of 'host_table' alias '__OFFLOAD_TABLE__', all
remaining host<->device function and/or variable assignments are messed up.

[Bug middle-end/109128] [Offload][OpenMP][OpenACC] Static linking with unused offload function will lead to mismatch number of offload fn/symbols

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109128

--- Comment #3 from Tobias Burnus <burnus at gcc dot gnu.org> ---
My initial thought was to handle it via lto1. This works well if all relevant
files are compiled with "-flto" as then the callers of the offload functions,
the offload functions themselves are available, permitting to generate
__OFFLOAD_TABLE__ directly.

However, if -flto is not used  or not used for all translation units (with
offload code), this approach will fail due to visibility problems.

Namely, the offload functions have local binding. This could be solved by
forcing global binding (with visibility hidden), but this approach will fail if
the assembler name is not unique.

 * * *

Thus, placing the symbols into a section will work (current behavior), except
that the linker might remove symbols (this bug) and one needs to hope that the
linking order is the same on the host as it is with LTO.

Still remains the question how to handle this PR...

[Bug middle-end/109128] [Offload][OpenMP][OpenACC] Static linking with unused offload function will lead to mismatch number of offload fn/symbols

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109128

--- Comment #4 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Minor observations (unrelated to this bug but should be fixed. separately or
together):

- '@' file in lto-wrapper.cc's compile_offload_image's fork_execute call is
always the same. (Matters if there is more than one offloading target
configured)

- The sections ".gnu.offload_funcs" and ".gnu.offload_vars" are 'wx' but they
could be 'x' (i.e. read only as the function addresses do not need to be
written to). → TREE_READONLY (funcs_decl) = 1; (same for 'vars_decl').

* * *

Tried the following, but it did not help – 'ld' does not import any symbol from
the 'libone.a' library – neither before nor after the patch; not did
-no-as-needed help and -Wl,--print-gc-sections does not show any discarded
sections.


diff --git a/gcc/omp-offload.cc b/gcc/omp-offload.cc
index 3bd144e9ccf..e1b22729994 100644
--- a/gcc/omp-offload.cc
+++ b/gcc/omp-offload.cc
@@ -437,0 +438,13 @@ omp_finish_file (void)
+      TREE_READONLY (funcs_decl) = 1;
+      TREE_READONLY (vars_decl) = 1;
+#ifndef ACCEL_COMPILER
+      if (SUPPORTS_SHF_GNU_RETAIN)
+       {
+         DECL_ATTRIBUTES (funcs_decl) = tree_cons (get_identifier ("retain"),
+                                                   NULL_TREE, NULL_TREE);
+         DECL_ATTRIBUTES (vars_decl) = tree_cons (get_identifier ("retain"),
+                                                  NULL_TREE, NULL_TREE);
+       }
+      else
+       sorry ("support for %<SHF_GNU_RETAIN%> required for offloading");
+#endif
diff --git a/libgcc/offloadstuff.c b/libgcc/offloadstuff.c
index 4e1c4d41dd5..59e191c5e19 100644
--- a/libgcc/offloadstuff.c
+++ b/libgcc/offloadstuff.c
@@ -51 +51 @@ const void *const __offload_func_table[0]
-  __attribute__ ((__used__, visibility ("hidden"),
+  __attribute__ ((__used__, visibility ("hidden"), retain,
@@ -54 +54 @@ const void *const __offload_var_table[0]
-  __attribute__ ((__used__, visibility ("hidden"),
+  __attribute__ ((__used__, visibility ("hidden"), retain,
@@ -60 +60 @@ const void *const __offload_funcs_end[0]
-  __attribute__ ((__used__, visibility ("hidden"),
+  __attribute__ ((__used__, visibility ("hidden"), retrain,
@@ -63 +63 @@ const void *const __offload_vars_end[0]
-  __attribute__ ((__used__, visibility ("hidden"),
+  __attribute__ ((__used__, visibility ("hidden"), retrain,


* * *

What's interesting is the file -foffload-objects=a.ofldlist - produced by
lto-plugin:

* If there is a common block in 'one.f90' (and via module use also in
'two.f90'), the a.ofldlist file contains both 'two.o' and the ./libone.a@0xc0
library.

* But if there is no common block in 'one.f90' (and hence also not in
'two.f90'), the a.ofldlist file contains only 'two.o'.

[Bug middle-end/109128] [Offload][OpenMP][OpenACC] Static linking with unused offload function will lead to mismatch number of offload fn/symbols

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109128

--- Comment #5 from Tobias Burnus <burnus at gcc dot gnu.org> ---
New idea for the lto-plugin.c:

Currently, as soon as a file pops up (as seemingly here for the common block in
libone.a), the file is claimed via claim_file_handler, which collects the file
names.

Later, in all_symbols_read_handler it is output. I wonder whether some check
using get_symbols is needed to check whether there is any symbol used for that
file.

[Bug middle-end/109128] [Offload][OpenMP][OpenACC] Static linking with unused offload function will lead to mismatch number of offload fn/symbols

[tschwinge at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109128

Thomas Schwinge <tschwinge at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |burnus at gcc dot gnu.org
             Status|NEW                         |ASSIGNED

--- Comment #6 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
I'm very likely missing some crucial "minor detail" here:

(In reply to Tobias Burnus from comment #3)
> My initial thought was to handle it via lto1. This works well if all
> relevant files are compiled with "-flto" as then the callers of the offload
> functions, the offload functions themselves are available, permitting to
> generate __OFFLOAD_TABLE__ directly.

I don't understand why '-flto', that is, why looking at *LTO data*.  My idea
had been that we'd have a mode for host-side 'lto1' where it reads the
*offloading data*.

(I'd, by the way think, that having a separate 'offload1' instead of
piggy-backing offloading handling on 'lto1' might be clearer generally --
especially once we get to actual offloading-LTO?)

> However, if -flto is not used  or not used for all translation units (with
> offload code), this approach will fail due to visibility problems.
> 
> Namely, the offload functions have local binding. This could be solved by
> forcing global binding (with visibility hidden), but this approach will fail
> if the assembler name is not unique.

That is, my understanding was that the *offloading data* contains all the
information that we need?

[Bug middle-end/109128] [Offload][OpenMP][OpenACC] Static linking with unused offload function will lead to mismatch number of offload fn/symbols

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109128

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> ---
RFC with patch to extend the linker plugin API:

linker side: https://sourceware.org/pipermail/binutils/2023-May/127271.html
GCC side: https://gcc.gnu.org/pipermail/gcc-patches/2023-May/617277.html

[Bug middle-end/109128] [Offload][OpenMP][OpenACC] Static linking with unused offload function will lead to mismatch number of offload fn/symbols

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109128

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tobias Burnus <burnus@gcc.gnu.org>:

https://gcc.gnu.org/g:c49d51fa8134f6c7e6c7cf6e4e3007c4fea617c5

commit r14-677-gc49d51fa8134f6c7e6c7cf6e4e3007c4fea617c5
Author: Joseph Myers <joseph@codesourcery.com>
Date:   Thu May 11 10:07:00 2023 +0200

    Implement LDPT_REGISTER_CLAIM_FILE_HOOK_V2 linker plugin hook [PR109128]

    This is one part of the fix for PR109128, along with a corresponding
    binutils's linker change.  Without this patch, what happens in the
    linker, when an unused object in a .a file has offload data, is that
    elf_link_is_defined_archive_symbol calls bfd_link_plugin_object_p,
    which ends up calling the plugin's claim_file_handler, which then
    records the object as one with offload data. That is, the linker never
    decides to use the object in the first place, but use of this _p
    interface (called as part of trying to decide whether to use the
    object) results in the plugin deciding to use its offload data (and a
    consequent mismatch in the offload data present at runtime).

    The new hook allows the linker plugin to distinguish calls to
    claim_file_handler that know the object is being used by the linker
    (from ldmain.c:add_archive_element), from calls that don't know it's
    being used by the linker (from elf_link_is_defined_archive_symbol); in
    the latter case, the plugin should avoid recording the object as one
    with offload data.

            PR middle-end/109128

            include/
            * plugin-api.h (ld_plugin_claim_file_handler_v2)
            (ld_plugin_register_claim_file_v2)
            (LDPT_REGISTER_CLAIM_FILE_HOOK_V2): New.
            (struct ld_plugin_tv): Add tv_register_claim_file_v2.

            lto-plugin/
            * lto-plugin.c (register_claim_file_v2): New.
            (claim_file_handler_v2): New.
            (claim_file_handler): Wrap claim_file_handler_v2.
            (onload): Handle LDPT_REGISTER_CLAIM_FILE_HOOK_V2.

[Bug middle-end/109128] [Offload][OpenMP][OpenACC] Static linking with unused offload function will lead to mismatch number of offload fn/symbols

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109128

Tobias Burnus <burnus at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Tobias Burnus <burnus at gcc dot gnu.org> ---
Close as FIXED.

The solution was to extend the linker plugin API to be able to tell GCC's
linker plugin whether a file is actually needed or is only passed to see
whether it contains by chance LTO symbols.

For the GCC patch that was applied to GCC mainline (GCC 14), see previous
comment (comment 8).

For the Binutils' patch, applied to mainline = Binutils 2.40.50,  see

https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=b21318bd2c29fcca8f99c1de7facdaa5cb2e66e2