[Bug target/70825] New: x86_64: __atomic_compare_exchange_n() accesses stack unnecessarily

[dhowells at redhat dot com]

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

            Bug ID: 70825
           Summary: x86_64: __atomic_compare_exchange_n() accesses stack
                    unnecessarily
           Product: gcc
           Version: 6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dhowells at redhat dot com
  Target Milestone: ---

Created attachment 38349
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=38349&action=edit
Example code

__atomic_compare_exchange_n() stores the 'expected' value and, if not
discarded, the old value on the stack rather than in a register and then may
immediately retrieve the old value from there back into the register from
whence it just came.

For example:

   static __always_inline int atomic_cmpxchg(atomic_t *v, int old, int new)
   {
      int cur = old;
      if (__atomic_compare_exchange_n(&v->counter, &cur, new, false,
                  __ATOMIC_SEQ_CST,
                  __ATOMIC_RELAXED))
         return cur;
      return cur;
   }

   void test_atomic_cmpxchg(atomic_t *counter)
   {
      atomic_cmpxchg(counter, 23, 42);
   }

produces:

   0:   ba 2a 00 00 00          mov    $0x2a,%edx
   5:   b8 17 00 00 00          mov    $0x17,%eax
   a:   c7 44 24 fc 17 00 00    movl   $0x17,-0x4(%rsp)
  11:   00 
  12:   f0 0f b1 17             lock cmpxchg %edx,(%rdi)
  16:   c3                      retq   

Note line a where 0x17 is stored at -04(%rsp) unnecessarily (the value is
dicarded).  Note also that this value is in %eax at the time and could be saved
from there rather than using an immediate operand.

And then:

   int test_atomic_cmpxchg_2(atomic_t *counter)
   {
      return atomic_cmpxchg(counter, 23, 42);
   }

where the value is returned, we get something like:

  20:   ba 2a 00 00 00          mov    $0x2a,%edx
  25:   b8 17 00 00 00          mov    $0x17,%eax
  2a:   c7 44 24 fc 17 00 00    movl   $0x17,-0x4(%rsp)
  31:   00 
  32:   f0 0f b1 17             lock cmpxchg %edx,(%rdi)
  36:   74 04                   je     3c <test_atomic_cmpxchg_2+0x1c>
  38:   89 44 24 fc             mov    %eax,-0x4(%rsp)
  3c:   8b 44 24 fc             mov    -0x4(%rsp),%eax
  40:   c3                      retq   

In this case, the return value is being constructed on the stack whereas the
value we actually want to return is in %eax at the conclusion of the CMPXCHG,
so lines 2a & 36-3c are redundant.

Changing atomic_cmpxchg() slightly:

  static __always_inline int atomic_cmpxchg_B(atomic_t *v, int old, int new)
   {
      int cur = old;
      if (__atomic_compare_exchange_n(&v->counter, &cur, new, false,
                  __ATOMIC_SEQ_CST,
                  __ATOMIC_RELAXED))
-->      return old;
      return cur;
   }

does generate code that's somewhat better:

  a0:   ba 2a 00 00 00          mov    $0x2a,%edx
  a5:   b8 17 00 00 00          mov    $0x17,%eax
  aa:   c7 44 24 fc 17 00 00    movl   $0x17,-0x4(%rsp)
  b1:   00 
  b2:   f0 0f b1 17             lock cmpxchg %edx,(%rdi)
  b6:   ba 17 00 00 00          mov    $0x17,%edx
  bb:   0f 45 d0                cmovne %eax,%edx
  be:   89 d0                   mov    %edx,%eax
  c0:   c3                      retq   

However, given the fact that old == cur must hold true if the CMPXCHG
succeeded,  lines b6-be are redundant. Note that the unnecessary store (line
aa) is still there.

Note that clang does somewhat better than gcc:

  50:   b9 2a 00 00 00          mov    $0x2a,%ecx
  55:   b8 17 00 00 00          mov    $0x17,%eax
  5a:   f0 0f b1 0f             lock cmpxchg %ecx,(%rdi)
  5e:   c3                      retq   
  5f:   90                      nop

See the attached example code which should be compiled to a .s file.

This is the case for all of:

    gcc version 5.3.1 20151207 (Red Hat 5.3.1-2) (GCC)
    gcc version 6.0.0 20160219 (Red Hat Cross 6.0.0-0.1) (GCC)
    gcc version 4.8.5 20150623 (Red Hat 4.8.5-2.x) (GCC)