[Bug target/104338] New: RISC-V: Subword atomics result in library calls

[Bug target/104338] New: RISC-V: Subword atomics result in library calls

[palmer at gcc dot gnu.org]

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

            Bug ID: 104338
           Summary: RISC-V: Subword atomics result in library calls
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: palmer at gcc dot gnu.org
  Target Milestone: ---

There's a handful of bugs sort of related to this one, but nothing specific. 
This has been a long-standing issue and I think folks are generally familiar
with it, but just to summarize: we don't have sub-word atomic instruction in
RISC-V, so we just call out to the libatomic routines.  This causes fallout in
a handful of places (see 86005 and 81358, for example) and there's been some
attempts to resolve it but nothing appears to have stuck.

I figured it'd be a good starter project for Patrick, as he's yet to do any GCC
stuff.  He's working through it and doesn't have anything to post yet, but
figured I'd just open the bug now so folks knew what was going on from our end.

[Bug target/104338] RISC-V: Subword atomics result in library calls

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=89835
   Last reconfirmed|                            |2022-02-01
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Bug 89835 comment #2 references this.

[Bug target/104338] RISC-V: Subword atomics result in library calls

[patrick at rivosinc dot com]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=89835
   Last reconfirmed|                            |2022-02-01
             Status|UNCONFIRMED                 |NEW

Patrick O'Neill <patrick at rivosinc dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |patrick at rivosinc dot com
   Last reconfirmed|2022-02-01 00:00:00         |
     Ever confirmed|1                           |0
             Status|NEW                         |UNCONFIRMED

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Bug 89835 comment #2 references this.

[Bug target/104338] RISC-V: Subword atomics result in library calls

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
           Severity|normal                      |enhancement
   Last reconfirmed|                            |2022-02-01

[Bug target/104338] RISC-V: Subword atomics result in library calls

[schwab@linux-m68k.org]

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

--- Comment #2 from Andreas Schwab <schwab@linux-m68k.org> ---
This also causes a lot of packages to fail to build with undefined references
to the libatomic libcalls.

[Bug target/104338] RISC-V: Subword atomics result in library calls

[patrick at rivosinc dot com]

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

--- Comment #3 from Patrick O'Neill <patrick at rivosinc dot com> ---
Created attachment 52368
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52368&action=edit
Initial patch

Patch submitted to mailing list.
Subject: [PATCH v1] RISC-V: Add support for inlining subword atomic operations.

[Bug target/104338] RISC-V: Subword atomics result in library calls

[i at rvalue dot moe]

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

rvalue <i at rvalue dot moe> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |i at rvalue dot moe

--- Comment #4 from rvalue <i at rvalue dot moe> ---
In short term, maybe we can change the spec to link against libatomic by
default (implemented in
https://github.com/riscv-collab/riscv-gcc/commit/2c4857d0981501b7c50bbf228de9e287611f8ae5).
It will solve a lot of build errors if we revert the value of `LIB_SPEC`
instead of only link against libatomic when `-pthread` is present.

Detailed talk about this: https://github.com/riscv-collab/riscv-gcc/issues/337

[Bug target/104338] RISC-V: Subword atomics result in library calls

[palmer at gcc dot gnu.org]

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

--- Comment #5 from palmer at gcc dot gnu.org ---
(In reply to rvalue from comment #4)
> In short term, maybe we can change the spec to link against libatomic by
> default (implemented in
> https://github.com/riscv-collab/riscv-gcc/commit/
> 2c4857d0981501b7c50bbf228de9e287611f8ae5). It will solve a lot of build
> errors if we revert the value of `LIB_SPEC` instead of only link against
> libatomic when `-pthread` is present.
> 
> Detailed talk about this:
> https://github.com/riscv-collab/riscv-gcc/issues/337

We talked through some options like that and decided it was too risky for
GCC-12.  We already found one ABI break related to this (see 84568), and want
to make sure we give distros adequate advance notice before something that we
know to break ABIs.

That said, it's really not a GCC ABI break, it's a per-package configure issue.
 We can fix the libstdcxx fallout, which is the only bit we know about right
now (though it's not like we've scrubbed builds for this).  If the folks
building distros think it's better to risk the ABI breaks rather than chase
around the build failures, then I'm fine rushing something in to GCC-12.

I see Andreas is already here, I'm having some trouble adding anyone else
though (I can never quite figure out Bugzilla...).

[Bug target/104338] RISC-V: Subword atomics result in library calls

[palmer at gcc dot gnu.org]

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

palmer at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kito.cheng at gmail dot com

--- Comment #6 from palmer at gcc dot gnu.org ---
Kito pointed out earlier today that it should already be possible to default to
libatomic via a --with-specs=... configure-time argument already, so one option
here would be to just add an example/reference spec to GCC.  That would allow
distros to opt in to the "always link libatomic" behavior, if they want to risk
the ABI-related issues like we see in libstdcxx (which we'd of course have to
fix).  It doesn't sort out the long-tail issues related to ABI compatibility
between GCC and LLVM (and the suggested mappings), but at least it gives folks
a unified mechanism for doing this.

I know it's pretty late, but that seems like something we could do on the
GCC-12 timeline.  It seems like the distro folks are pretty fed up with waiting
so they're just going to backport/hack this if we miss GCC-12, might as well
have one way for that to happen.

[Bug target/104338] RISC-V: Subword atomics result in library calls

[c141028 at gmail dot com]

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

SS JieJi <c141028 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |c141028 at gmail dot com

--- Comment #7 from SS JieJi <c141028 at gmail dot com> ---
FYI Patch v2 has been submitted in the mailing list, see
https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg281336.html

I'm not sure why bugzilla lost sync with the mailing list.

[Bug target/104338] RISC-V: Subword atomics result in library calls

[patrick at rivosinc dot com]

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

Patrick O'Neill <patrick at rivosinc dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #52368|0                           |1
        is obsolete|                            |

--- Comment #8 from Patrick O'Neill <patrick at rivosinc dot com> ---
Created attachment 52835
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52835&action=edit
Patch v3

Patch submitted to mailing list.
Subject: [PATCH v3] RISC-V: Add support for inlining subword atomic operations.

[Bug target/104338] RISC-V: Subword atomics result in library calls

[redi at gcc dot gnu.org]

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

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to RZ Pan (XieJiSS) from comment #7)
> I'm not sure why bugzilla lost sync with the mailing list.

There is no sync between gcc-patches and bugzilla, and never has been.

[Bug target/104338] RISC-V: Subword atomics result in library calls

[aurelien at aurel32 dot net]

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

Aurelien Jarno <aurelien at aurel32 dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |aurelien at aurel32 dot net

--- Comment #10 from Aurelien Jarno <aurelien at aurel32 dot net> ---
I am not subscribed to the mailing list, but I am not able to see any more
mails after the v3. Any update?

[Bug target/104338] RISC-V: Subword atomics result in library calls

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |raj.khem at gmail dot com

--- Comment #11 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 108564 has been marked as a duplicate of this bug. ***

[Bug target/104338] RISC-V: Subword atomics result in library calls

[palmer at gcc dot gnu.org]

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

--- Comment #12 from palmer at gcc dot gnu.org ---
I've got a somewhat recently rebased version of Patrick's patch floating
around, it passed testing but I got hung up on the futex_time64 thing and
forgot about it.  Not sure if folks think it's too late for the upcoming CGC
release, but I wouldn't be opposed to taking it -- looks like distros aro going
to apply workarounds if we don't do something, so at least this way there'll be
a single workaround in trunk.

There's some bigger fixes in the works for the whole memory model as we've got
other issues, but since those are a bit tricker it might be worth just doing
the stop-gap thing for now.

[Bug target/104338] RISC-V: Subword atomics result in library calls

[burnus at gcc dot gnu.org]

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

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

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

--- Comment #13 from Tobias Burnus <burnus at gcc dot gnu.org> ---
I see v3 of the patch at
https://gcc.gnu.org/pipermail/gcc-patches/2022-April/593378.html

And v4 at
  https://gcc.gnu.org/pipermail/gcc-patches/2022-October/604584.html
with last reply at
  https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606366.html

[Bug target/104338] RISC-V: Subword atomics result in library calls

[patrick at rivosinc dot com]

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

--- Comment #14 from Patrick O'Neill <patrick at rivosinc dot com> ---
I picked this back up, v7 is here:
https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616080.html

[Bug target/104338] RISC-V: Subword atomics result in library calls

[cvs-commit at gcc dot gnu.org]

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

--- Comment #15 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick O'Neill <poneill@gcc.gnu.org>:

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

commit r14-269-gf797260adaf52bee0ec0e16190bbefbe1bfc3692
Author: Patrick O'Neill <patrick@rivosinc.com>
Date:   Tue Apr 18 14:33:13 2023 -0700

    RISCV: Inline subword atomic ops

    RISC-V has no support for subword atomic operations; code currently
    generates libatomic library calls.

    This patch changes the default behavior to inline subword atomic calls
    (using the same logic as the existing library call).
    Behavior can be specified using the -minline-atomics and
    -mno-inline-atomics command line flags.

    gcc/libgcc/config/riscv/atomic.c has the same logic implemented in asm.
    This will need to stay for backwards compatibility and the
    -mno-inline-atomics flag.

    2023-04-18 Patrick O'Neill <patrick@rivosinc.com>

    gcc/ChangeLog:
            PR target/104338
            * config/riscv/riscv-protos.h: Add helper function stubs.
            * config/riscv/riscv.cc: Add helper functions for subword masking.
            * config/riscv/riscv.opt: Add command-line flag.
            * config/riscv/sync.md: Add masking logic and inline asm for
fetch_and_op,
            fetch_and_nand, CAS, and exchange ops.
            * doc/invoke.texi: Add blurb regarding command-line flag.

    libgcc/ChangeLog:
            PR target/104338
            * config/riscv/atomic.c: Add reference to duplicate logic.

    gcc/testsuite/ChangeLog:
            PR target/104338
            * gcc.target/riscv/inline-atomics-1.c: New test.
            * gcc.target/riscv/inline-atomics-2.c: New test.
            * gcc.target/riscv/inline-atomics-3.c: New test.
            * gcc.target/riscv/inline-atomics-4.c: New test.
            * gcc.target/riscv/inline-atomics-5.c: New test.
            * gcc.target/riscv/inline-atomics-6.c: New test.
            * gcc.target/riscv/inline-atomics-7.c: New test.
            * gcc.target/riscv/inline-atomics-8.c: New test.

    Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
    Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>

[Bug target/104338] RISC-V: Subword atomics result in library calls

[cvs-commit at gcc dot gnu.org]

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

--- Comment #16 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Patrick O'Neill
<poneill@gcc.gnu.org>:

https://gcc.gnu.org/g:55088cf384d4c43e280ec794217e32fab64070ba

commit r13-7336-g55088cf384d4c43e280ec794217e32fab64070ba
Author: Patrick O'Neill <patrick@rivosinc.com>
Date:   Tue Apr 18 14:33:13 2023 -0700

    RISCV: Inline subword atomic ops

    RISC-V has no support for subword atomic operations; code currently
    generates libatomic library calls.

    This patch changes the default behavior to inline subword atomic calls
    (using the same logic as the existing library call).
    Behavior can be specified using the -minline-atomics and
    -mno-inline-atomics command line flags.

    gcc/libgcc/config/riscv/atomic.c has the same logic implemented in asm.
    This will need to stay for backwards compatibility and the
    -mno-inline-atomics flag.

    2023-05-03 Patrick O'Neill <patrick@rivosinc.com>

    gcc/ChangeLog:
            PR target/104338
            * config/riscv/riscv-protos.h: Add helper function stubs.
            * config/riscv/riscv.cc: Add helper functions for subword masking.
            * config/riscv/riscv.opt: Add command-line flags
            -minline-atomics and -mno-inline-atomics.
            * config/riscv/sync.md: Add masking logic and inline asm for
            fetch_and_op, fetch_and_nand, CAS, and exchange ops.
            * doc/invoke.texi: Add blurb regarding new command-line flags
            -minline-atomics and -mno-inline-atomics.

    libgcc/ChangeLog:
            PR target/104338
            * config/riscv/atomic.c: Add reference to duplicate logic.

    gcc/testsuite/ChangeLog:
            PR target/104338
            * gcc.target/riscv/inline-atomics-1.c: New test.
            * gcc.target/riscv/inline-atomics-2.c: New test.
            * gcc.target/riscv/inline-atomics-3.c: New test.
            * gcc.target/riscv/inline-atomics-4.c: New test.
            * gcc.target/riscv/inline-atomics-5.c: New test.
            * gcc.target/riscv/inline-atomics-6.c: New test.
            * gcc.target/riscv/inline-atomics-7.c: New test.
            * gcc.target/riscv/inline-atomics-8.c: New test.

    Signed-off-by: Patrick O'Neill <patrick@rivosinc.com>
    Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>

[Bug target/104338] RISC-V: Subword atomics result in library calls

[patrick at rivosinc dot com]

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

Patrick O'Neill <patrick at rivosinc dot com> changed:

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

--- Comment #17 from Patrick O'Neill <patrick at rivosinc dot com> ---
Closing this issue since the fix is on trunk and was backported to
releases/gcc-13 to be included in the next release.

[Bug target/104338] RISC-V: Subword atomics result in library calls

[aurelien at aurel32 dot net]

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

--- Comment #18 from Aurelien Jarno <aurelien at aurel32 dot net> ---
Thanks Patrick for working on that and for the backport to the gcc-13 branch. I
wonder if the following patch should also be backported, as it doesn't make
sense to link with -latomic anymore with inline subword atomic operations:

commit 203f3060dd363361b172f7295f42bb6bf5ac0b3b
Author: Andreas Schwab <schwab@suse.de>
Date:   Sat Apr 23 15:48:42 2022 +0200

    riscv/linux: Don't add -latomic with -pthread

    Now that we have support for inline subword atomic operations, it is no
    longer necessary to link against libatomic.  This also fixes testsuite
    failures because the framework does not properly set up the linker flags
    for finding libatomic.
    The use of atomic operations is also independent of the use of libpthread.

    gcc/
            * config/riscv/linux.h (LIB_SPEC): Don't redefine.

[Bug target/104338] RISC-V: Subword atomics result in library calls

[i at rvalue dot moe]

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

--- Comment #19 from rvalue <i at rvalue dot moe> ---
(In reply to Aurelien Jarno from comment #18)
> I wonder if the following patch should also be backported, as it
> doesn't make sense to link with -latomic anymore with inline subword atomic
> operations

Agreed. It's now meaningless to keep this workaround for RISC-V as the problem
has been resolved.

[Bug target/104338] RISC-V: Subword atomics result in library calls

[palmer at gcc dot gnu.org]

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

--- Comment #20 from palmer at gcc dot gnu.org ---
(In reply to rvalue from comment #19)
> (In reply to Aurelien Jarno from comment #18)
> > I wonder if the following patch should also be backported, as it
> > doesn't make sense to link with -latomic anymore with inline subword atomic
> > operations
> 
> Agreed. It's now meaningless to keep this workaround for RISC-V as the
> problem has been resolved.

Yep.  Can someone send the backport?

[Bug target/104338] RISC-V: Subword atomics result in library calls

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |13.2