[Bug c++/109362] New: codegen adds unnecessary extra add when reading atomic member

[Bug c++/109362] New: codegen adds unnecessary extra add when reading atomic member

[barry.revzin at gmail dot com]

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

            Bug ID: 109362
           Summary: codegen adds unnecessary extra add when reading atomic
                    member
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: barry.revzin at gmail dot com
  Target Milestone: ---

This program:

#include <atomic>

struct S {
    long size;
    std::atomic<char*> read_ptr;

    auto peek() const -> const char* {    
        return read_ptr.load(std::memory_order_acquire);
    }
};

auto with_atomic(S const& v) {
    while (true) {
        if (v.peek()) {
            return true;
        }
    }
}

emits (on gcc 12.2 -O3):

with_atomic(S const&):
        add     rdi, 8
.L2:
        mov     rax, QWORD PTR [rdi]
        test    rax, rax
        je      .L2
        mov     eax, 1
        ret

But that add is completely necessary, the mov could just be:

        mov     rax, QWORD PTR [rdi + 8]

which is what clang (16.0 -O3) generates:

with_atomic(S const&):                    # @with_atomic(S const&)
.LBB0_1:                                # =>This Inner Loop Header: Depth=1
        mov     rax, qword ptr [rdi + 8]
        test    rax, rax
        je      .LBB0_1
        mov     al, 1
        ret

It's not just an extra add, it's consuming an extra register - which has more
downstream optimization effects.

[Bug c++/109362] codegen adds unnecessary extra add when reading atomic member

[barry.revzin at gmail dot com]

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

--- Comment #1 from Barry Revzin <barry.revzin at gmail dot com> ---
Sorry, in this reduced example, it doesn't actually consume an extra register -
only rdi is used. 

In this slightly less reduced example:

#include <atomic>

struct S {
    std::atomic<long> size;
    std::atomic<char*> read_ptr;

    auto peek() const -> const char* {    
        auto const s = size.load(std::memory_order_acquire);
        return read_ptr.load(std::memory_order_acquire);
    }
};

auto with_atomic(S const& v) {
    while (true) {
        if (v.peek()) {
            return true;
        }
    }
}


the codegen is:

with_atomic(WithAtomic const&):
        lea     rdx, [rdi+8]
.L2:
        mov     rax, QWORD PTR [rdi]
        mov     rax, QWORD PTR [rdx]
        test    rax, rax
        je      .L2
        mov     eax, 1
        ret

which now does consume rdx unnecessarily.

[Bug c++/109362] codegen adds unnecessary extra add when reading atomic member

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This got fixed with r13-137-gee1cb43bc76de800efa0ade6 on the trunk.

[Bug c++/109362] codegen adds unnecessary extra add when reading atomic member

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Reduced C testcase would be
struct S { long a, b; };

int
foo (struct S *v)
{
  while (1)
    {
      __atomic_load_n (&v->a, __ATOMIC_ACQUIRE);
      if (__atomic_load_n (&v->b, __ATOMIC_ACQUIRE))
        return 1;
    }
}

[Bug c++/109362] codegen adds unnecessary extra add when reading atomic member

[barry.revzin at gmail dot com]

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

--- Comment #4 from Barry Revzin <barry.revzin at gmail dot com> ---
Awesome!

[Bug tree-optimization/109362] codegen adds unnecessary extra add when reading atomic member

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:87d3bc53b177037699f7f8dda3a3d17e647c459d

commit r13-6966-g87d3bc53b177037699f7f8dda3a3d17e647c459d
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Apr 1 09:00:22 2023 +0200

    testsuite: Add testcase for already fixed PR [PR109362]

    This PR got fixed with r13-137.
    Add a testcase to make sure it doesn't reappear.

    2023-04-01  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/109362
            * gcc.target/i386/pr109362.c: New test.