[Bug other/113336] New: libatomic (testsuite) regressions on armv6-linux-gnueabihf

[Bug other/113336] New: libatomic (testsuite) regressions on armv6-linux-gnueabihf

[roger at nextmovesoftware dot com]

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

            Bug ID: 113336
           Summary: libatomic (testsuite) regressions on
                    armv6-linux-gnueabihf
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: other
          Assignee: unassigned at gcc dot gnu.org
          Reporter: roger at nextmovesoftware dot com
  Target Milestone: ---

As suggested by Richard Earnshaw, this opens a bugzilla PR for tracking this
issue.  All the tests in libatomic currently fail on a raspberry pi running
raspbian, but passed back in December 2020.
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/642168.html

The regression (which isn't really a regression) was caused by:
2023-09-26  Hans-Peter Nilsson  <hp@axis.com>

        PR target/107567
        PR target/109166
        * builtins.cc (expand_builtin) <case BUILT_IN_ATOMIC_TEST_AND_SET>:
        Handle failure from expand_builtin_atomic_test_and_set.
        * optabs.cc (expand_atomic_test_and_set): When all attempts fail to
        generate atomic code through target support, return NULL
        instead of emitting non-atomic code.  Also, for code handling
        targetm.atomic_test_and_set_trueval != 1, gcc_assert result
        from calling emit_store_flag_force instead of returning NULL.


Prior to this, when -fno-sync-libcalls was specified on the command line, the
__atomic_test_and_set built-in simply expanded to a non-atomic code sequence,
which then passed libatomic's configure tests for HAVE_ATOMIC_TAS.  Now that
this hole/bug/correctness issue has been fixed, and HAVE_ATOMIC_TAS is now
detected as false, the libatomics's tas_n.c can no longer implement tas_8_2_.o
without (a missing helper function) tas_1_2_.o.

Hence libatomic has (always?) been broken on armv6, but synchronization
primitives can now be supported with the above change. We've just not noticed
that necessary pieces of the runtime were missing, until the above correctness
fix resulted in a link error.

[Bug other/113336] libatomic (testsuite) regressions on armv6-linux-gnueabihf

[roger at nextmovesoftware dot com]

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

Roger Sayle <roger at nextmovesoftware dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2024-01-14
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1
           Assignee|unassigned at gcc dot gnu.org      |roger at nextmovesoftware dot com

--- Comment #1 from Roger Sayle <roger at nextmovesoftware dot com> ---
As there's a patch for this regression (awaiting review), I should upgrade the
PR status to ASSIGNED.

[Bug other/113336] libatomic (testsuite) regressions on armv6-linux-gnueabihf

[hp at gcc dot gnu.org]

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

Hans-Peter Nilsson <hp at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hp at gcc dot gnu.org

--- Comment #2 from Hans-Peter Nilsson <hp at gcc dot gnu.org> ---
Whoops, sorry! (no, not actually sorry :)  I'm glad you have a handle on this,
though!

[Bug other/113336] libatomic (testsuite) regressions on armv6-linux-gnueabihf

[victor.donascimento at arm dot com]

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

Victor Do Nascimento <victor.donascimento at arm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |victor.donascimento at arm dot com

--- Comment #3 from Victor Do Nascimento <victor.donascimento at arm dot com> ---
For what it's worth, I just so happened to stumble upon the same issue when
compiling and running Libatomic for the armv8l-unknown-linux-gnueabihf triplet
on a Cortex-A72 machine inside a 32-bit Docker container, so the issue is
clearly is clearly prevalent on a wider range of targets than perhaps alluded
to by the bug report title.

The patch provided does appear to fix all regressions.

Here are my initial thoughts on the issue and the proposed fix.

My only concern at the moment is that if the regression is caused by
HAVE_ATOMIC_TAS now being detected as false, then perhaps a more directed
solution is called for, specific to tas, as opposed to generating _i2 variants
for *all* atomic ops via $(addsuffix _1_2_.lo,$(SIZEOBJS))

If you look at the very end of tas_n.c at the `if !DONE' clause, you'll see
that for `SIZE(libat_test_and_set)', irrespective of the SIZE value,
SIZE(libat_test_and_set) always falls back onto `libat_test_and_set_1',
explaining why tas_1_2_.lo is needed.

This unconditional dependence on the *_1 does not, however, appear the norm. 
One example of this is seen with `SIZE(libat_compare_exchange)'.

With this in mind, I notice that adding `tas_1_2_.lo' to the
`libatomic_la_LIBADD' variable in Makefile.am, i.e.

  libatomic_la_LIBADD += tas_1_2_.lo

is apparently sufficient to fix all regressions on my machine.

[Bug other/113336] libatomic (testsuite) regressions on arm

[roger at nextmovesoftware dot com]

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

Roger Sayle <roger at nextmovesoftware dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |NEW
           Assignee|roger at nextmovesoftware dot com  |unassigned at gcc dot gnu.org
            Summary|libatomic (testsuite)       |libatomic (testsuite)
                   |regressions on              |regressions on arm
                   |armv6-linux-gnueabihf       |

--- Comment #4 from Roger Sayle <roger at nextmovesoftware dot com> ---
Hi Victor,
Yes, I agree your approach is better/less invasive than mine.  I simply copied
the existing idiom in Makefile.am, not noticing that this adds more
functionality to libatomic than is strictly required. Just adding the
missing/required tas_1_2_.lo is better (and hopefully more acceptable to the
maintainers/reviewers).

[Bug other/113336] libatomic (testsuite) regressions on arm

[doko at gcc dot gnu.org]

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

Matthias Klose <doko at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |doko at gcc dot gnu.org

--- Comment #5 from Matthias Klose <doko at gcc dot gnu.org> ---
see also PR113336, causes also build failure of gnat and libgo on
arm-linux-gnueabi

[Bug other/113336] [14 Regression] libatomic (testsuite) regressions on arm

[doko at gcc dot gnu.org]

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

--- Comment #6 from Matthias Klose <doko at gcc dot gnu.org> ---
*** Bug 113635 has been marked as a duplicate of this bug. ***

[Bug other/113336] [14 Regression] libatomic (testsuite) regressions on arm

[roger at nextmovesoftware dot com]

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

Roger Sayle <roger at nextmovesoftware dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |roger at nextmovesoftware dot com
   Target Milestone|---                         |14.0

--- Comment #7 from Roger Sayle <roger at nextmovesoftware dot com> ---
A revised patch has been posted for review/approval to gcc-patches:
https://gcc.gnu.org/pipermail/gcc-patches/2024-January/644147.html

[Bug other/113336] [14 Regression] libatomic (testsuite) regressions on arm

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Roger Sayle <sayle@gcc.gnu.org>:

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

commit r14-8988-gea7675761226b42e2deb7b320e9cc680512f4090
Author: Roger Sayle <roger@nextmovesoftware.com>
Date:   Wed Feb 14 19:09:51 2024 +0000

    PR other/113336: Fix libatomic testsuite regressions on ARM.

    This patch is a revised version of the fix for PR other/113336.
    Bootstrapping GCC on arm-linux-gnueabihf with --with-arch=armv6 currently
    has a large number of FAILs in libatomic (regressions since last time I
    attempted this).  The failure mode is related to IFUNC handling with the
    file tas_8_2_.o containing an unresolved reference to the function
    libat_test_and_set_1_i2.

    The following one line change, to build tas_1_2_.o when building
tas_8_2_.o,
    resolves the problem for me and restores the libatomic testsuite to 44
    expected passes and 5 unsupported tests [from 22 unexpected failures
    and 22 unresolved testcases].
    `

    2024-02-14  Roger Sayle  <roger@nextmovesoftware.com>
                Victor Do Nascimento  <victor.donascimento@arm.com>

    libatomic/ChangeLog
            PR other/113336
            * Makefile.am: Build tas_1_2_.o on ARCH_ARM_LINUX
            * Makefile.in: Regenerate.

[Bug other/113336] [14 Regression] libatomic (testsuite) regressions on arm

[roger at nextmovesoftware dot com]

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

Roger Sayle <roger at nextmovesoftware dot com> changed:

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

--- Comment #9 from Roger Sayle <roger at nextmovesoftware dot com> ---
This should now be fixed on mainline.