[Bug other/111966] New: GCN '--with-arch=[...]' not considered for 'mkoffload' default 'elf_arch'

[Bug other/111966] New: GCN '--with-arch=[...]' not considered for 'mkoffload' default 'elf_arch'

[tschwinge at gcc dot gnu.org]

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

            Bug ID: 111966
           Summary: GCN '--with-arch=[...]' not considered for 'mkoffload'
                    default 'elf_arch'
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Keywords: openacc, openmp
          Severity: normal
          Priority: P3
         Component: other
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tschwinge at gcc dot gnu.org
                CC: ams at gcc dot gnu.org, jules at gcc dot gnu.org
  Target Milestone: ---
            Target: GCN

I've noticed this issue long ago; it probably existed since commit
r11-2178-ga4f49061b6d921f72b2faf4843144f3c75f828f7 "amdgcn: Handle early debug
info in mkoffload", when GCN offloading GCC 'configure'd different from
(implicit or explicit) '--with-arch=fiji', and compiling user code without
explicit '-foffload-options=amdgcn-amdhsa=-march=[...]'.  In that case, the
compiler-side default doesn't match 'gcc/config/gcn/mkoffload.cc:uint32_t
elf_arch = EF_AMDGPU_MACH_AMDGCN_GFX803;', and 'elf_arch' isn't getting
corrected by an explicit '-march'.  In that case, you'll get offloading
compilation failures if debugging ('-g') is enabled, for example:

    [-PASS:-]{+FAIL:+} libgomp.fortran/declare-target-1.f90   -O3 -g  (test for
excess errors)
    [-PASS:-]{+UNRESOLVED:+} libgomp.fortran/declare-target-1.f90   -O3 -g 
[-execution test-]{+compilation failed to produce executable+}

... due to:

    [...]
    ld: error: incompatible mach: /tmp/cc8pa454.mkoffload.dbg.o
    collect2: error: ld returned 1 exit status
    gcn mkoffload: fatal error:
[...]/x86_64-pc-linux-gnu-accel-amdgcn-amdhsa-gcc returned 1 exit status
    [...]

('elf_arch' is used only in 'copy_early_debug_info'.)

Not a very typical configuration that anyone but me would have used ;-) -- but
I'm reporting it now, given that as of recent commit
r14-4734-g56ed1055b2f40ac162ae8d382280ac07a33f789f "amdgcn: deprecate Fiji
device and multilib", users are more likely to run into this: the compiler-side
default is now '--with-arch=gfx900' instead of 'fiji', and generally people may
specify a specific non-'fiji' '--with-arch=[...]' (for example, together with
'--without-multilib-list'), and build their code without explicit
'-foffload-options=amdgcn-amdhsa=-march=[...]'.

We need to propagate the compiler-side default '--with-arch=[...]' into
'mkoffload' 'elf_arch' -- or change the driver to simply always pass an
explicit '-march' to 'mkoffload' (and verify that, make the default 'elf_arch'
invalid)?

[Bug other/111966] GCN '--with-arch=[...]' not considered for 'mkoffload' default 'elf_arch'

[cvs-commit at gcc dot gnu.org]

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

--- Comment #1 from GCC 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:13127dac106724bef3a979539a878b368b79ce56

commit r14-8332-g13127dac106724bef3a979539a878b368b79ce56
Author: Tobias Burnus <tburnus@baylibre.com>
Date:   Mon Jan 22 12:17:12 2024 +0100

    [gcn] mkoffload: Fix linking with "-g"; fix file deletion; improve
diagnostic [PR111966]

    With debugging enabled, '*.mkoffload.dbg.o' files are generated. The
e_flags
    header of all *.o files must be the same - otherwise, the linker complains.
    Since r14-4734-g56ed1055b2f40ac162ae8d382280ac07a33f789f the -march=
default
    is now gfx900. If compiling without any -march= flag, the default value is
    used by the compiler but not passed to mkoffload. Hence, mkoffload.cc's
uses
    its own default for march - unfortunately, it still had gfx803/fiji as
default,
    leading to the linker error: 'incompatible mach'. Solution: Update the
    default to gfx900.

    While debugging it, I saw that /tmp/cc*.mkoffload.dbg.o kept accumulating;
    there were a couple of issues with the handling:
    * dbgobj was always added to files_to_cleanup
    * If copy_early_debug_info returned true, dbgobj was added again
      -> pointless and in theory a race if the same file was added in the
         faction of a second.
    * If copy_early_debug_info returned false,
      - In exactly one case, it already deleted the file it self
        (same potential race as above)
      - The pointer dbgobj was freed - such that files_to_cleanup contained
        a dangling pointer - probably the reason that stale files remained.
    Solution: Only if copy_early_debug_info returns true, dbgobj is added to
    files_to_cleanup. If it returns false, the file is unlinked before freeing
    the pointer.

    When compiling, GCC warned about several fatal_error messages as having
    no %<...%> or %qs quotes. This patch now silences several of those warnings
    by using those quotes.

    gcc/ChangeLog:

            PR other/111966
            * config/gcn/mkoffload.cc (elf_arch): Change default to gfx900
            to match the compiler default.
            (simple_object_copy_lto_debug_sections): Never unlink the outfile
            on error as the caller does so.
            (maybe_unlink, compile_native): Use %<...%> and %qs in fatal_error.
            (main): Likewise. Fix 'mkoffload.dbg.o' cleanup.

    Signed-off-by: Tobias Burnus <tburnus@baylibre.com>

[Bug other/111966] GCN '--with-arch=[...]' not considered for 'mkoffload' default 'elf_arch'

[burnus at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |burnus at gcc dot gnu.org
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #2 from Tobias Burnus <burnus at gcc dot gnu.org> ---
FIXED on mainline/GCC 14.

[Bug other/111966] GCN '--with-arch=[...]' not considered for 'mkoffload' default 'elf_arch'

[tschwinge at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|RESOLVED                    |REOPENED
   Last reconfirmed|                            |2024-01-24
         Resolution|FIXED                       |---

--- Comment #3 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
Tobias, thanks for fixing the easy part ('s%803%900' default) -- however, the
harder part still remains to be done; see this issue's initial comment.

[Bug other/111966] GCN '--with-arch=[...]' not considered for 'mkoffload' default 'elf_arch'

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from GCC 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:ef5ccdbbc60c230a483898afbf0c053a9f8bb176

commit r14-8489-gef5ccdbbc60c230a483898afbf0c053a9f8bb176
Author: Tobias Burnus <tburnus@baylibre.com>
Date:   Mon Jan 29 11:10:33 2024 +0100

    gcn/mkoffload.cc: Fix SRAM_ECC and XNACK handling [PR111966]

    Some more '-g' fixes as the .mkoffload.dbg.o debug file's has elf flags
    which did not match those generated for the compilation, leading to linker
    errors.  For .mkoffload.dbg.o, the elf flags are generated by mkoffload
    itself - while for the other .o files, that's done by the compiler via
    setting default and mainly via the ASM_SPEC.

    This is a follow up to r14-8332-g13127dac106724 which fixed an issue
    caused by the default arch.  In this patch, it is mainly for gfx1100
    and gfx1030 which always failed.  It also affects gfx906 and possibly
    gfx900 but only when using the -mxnack/-msram-ecc flags explicitly.

    What happens on the compiler side is mainly determined by gcn-hsa.h's
    and otherwise by some default setting. In particular for xnack and
    sram_ecc, there is:

    For gfx1100 and gfx1030, neither xnack nor sram_ecc is set (only
    '+wavefrontsize64').

    For fiji, gfx900, gfx906 and gfx908 there is always -mattr=-xnack and
    for all but gfx908 also -msram-ecc=no - independent of what has been
    passed to the compiler. However, on the elf flags, the result differs:
    For fiji, due to the HSACOv3, it is always set to 0 via
    copy_early_debug_info; for gfx900, gfx906 and gfx908, xnack is OFF.
    For sram-ecc, it is 'unset' for gfx900, 'any' for gfx906 and for
    gfx908 it is 'any' unless overridden.

    For gfx90a, the -msram-ecc= and -mxnack= are passed on, or if not present,
    ...=any is passed on.  Note that this "any" is different from argument
    nor present at elf flag level:
    For XNACK: unset/unsupported is 0, any = 0x100, off = 0x200, on = 0x300.
    For SRAMECC: unset/unsupported is 0, any = 0x400, off = 0x800, on = 0xc00.

    The obstack_ptr_grow changes are more to avoid confusion than having an
    actual effect as they would overwise be filtered out via the ASM_SPEC.

    gcc/ChangeLog:

            PR other/111966
            * config/gcn/mkoffload.cc (SET_XNACK_UNSET, TEST_SRAM_ECC_UNSET):
New.
            (SET_SRAM_ECC_UNSUPPORTED): Renamed to ...
            (SET_SRAM_ECC_UNSET): ... this.
            (copy_early_debug_info): Remove gfx900 special case, now handled as
            part of the generic handling.
            (main): Update SRAM_ECC and XNACK for the -march as done in
gcn-hsa.h.

    Signed-off-by: Tobias Burnus <tburnus@baylibre.com>

[Bug other/111966] GCN '--with-arch=[...]' not considered for 'mkoffload' default 'elf_arch'

[tschwinge at gcc dot gnu.org]

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

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
                 CC|                            |rguenth at gcc dot gnu.org
             Status|REOPENED                    |ASSIGNED

--- Comment #5 from Thomas Schwinge <tschwinge at gcc dot gnu.org> ---
Tobias is working on this.

[Bug other/111966] GCN '--with-arch=[...]' not considered for 'mkoffload' default 'elf_arch'

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from GCC 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:b2460d621efe740bd95ad41afef6d806ec1bd9c7

commit r14-9770-gb2460d621efe740bd95ad41afef6d806ec1bd9c7
Author: Tobias Burnus <tburnus@baylibre.com>
Date:   Wed Apr 3 12:37:39 2024 +0200

    GCN: Fix --with-arch= handling in mkoffload [PR111966]

    The default -march= setting used in mkoffload did not reflect the modified
    default set by GCC's configure-time --with-arch=, causing issues when
    generating debug code.

    gcc/ChangeLog:

            PR other/111966
            * config/gcn/mkoffload.cc (get_arch): New; moved -march= flag
            handling from ...
            (main): ... here; call it to handle --with-arch config option
            and -march= commandline.

[Bug other/111966] GCN '--with-arch=[...]' not considered for 'mkoffload' default 'elf_arch'

[burnus at gcc dot gnu.org]

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

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

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

--- Comment #7 from Tobias Burnus <burnus at gcc dot gnu.org> ---
FIXED on mainline (= GCC 14).

Namely, the following was fixed. All of those issues involve compiling with
'-g' such that 'mkoffload' generates also a GCN .o file for which the ELF flag
has to match the other .o files.

(A) The issue of comment 0: ELF Flag mismatch if GCC was configured with a
--with-arch=... that does not match the default setting.
→ Fix: See comment 6

Earlier fixes, only vaguely related to comment 0:

(B)
* Compiler default was changed to gfx900 but mkoffload still had Fiji as
default
* Race in handling the debug files
→ Fix: See comment 1

(C)
* Fixed issues related to xnack/sram-ecc, which also lead to ELF flag
mismatches
→ Fix: See comment 4