[Bug middle-end/58330] New: powerpc64 atomic store split in two

[Bug middle-end/58330] New: powerpc64 atomic store split in two

[anton at samba dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58330

            Bug ID: 58330
           Summary: powerpc64 atomic store split in two
           Product: gcc
           Version: 4.8.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: anton at samba dot org

We found an issue when building the powerpc64 little endian kernel where a 64
bit store was split into two 32 bit stores. Since there is no 64 bit byte
reversed store on POWER6 and earlier, gcc decided to use two 32 bit byte
reversed stores. We were unable to fix this, even with atomic builtins.

I think splitting the store even in the non atomic builtin case is suspect
since we have lots of places in the kernel that rely on naturally aligned loads
and stores being done atomically.

An example:

static inline void __raw_writeq (unsigned long v, void *addr)
{
#if 1
        __atomic_store_n ((unsigned long *) addr, v, __ATOMIC_RELAXED);
#else
        *(volatile unsigned long *)addr = v;
#endif
}

void foo (void *addr, unsigned long start)
{
        unsigned long reverse = __builtin_bswap64 (start);
        __raw_writeq (reverse, addr);
}

gives:

    li 9,4
    srdi 10,4,32
    stwbrx 4,0,3
    stwbrx 10,9,3

[Bug target/58330] powerpc64 atomic store split in two

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58330

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |powerpc64*-linux
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-09-06
          Component|middle-end                  |target
     Ever confirmed|0                           |1

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I think the rs6000 back-end should follow what aarch64 does with respect of the
atomic stores and don't use a standard movdi pattern but have an atomic store
pattern instead.

[Bug target/58330] powerpc64 atomic store split in two

[amodra at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58330

Alan Modra <amodra at gmail dot com> changed:

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

--- Comment #2 from Alan Modra <amodra at gmail dot com> ---
I think this is a backend problem, specifically with bswapdi2_64bit accepting a
DImode mem then splitting to two SImode.

Potential fix is to add
   && !(MEM_P (operands[0]) && MEM_VOLATILE_P (operands[0]))
   && !(MEM_P (operands[1]) && MEM_VOLATILE_P (operands[1]))
to the insn condition.

[Bug target/58330] powerpc64 atomic store split in two

[amodra at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58330

--- Comment #3 from Alan Modra <amodra at gcc dot gnu.org> ---
Author: amodra
Date: Mon Sep  9 23:48:10 2013
New Revision: 202418

URL: http://gcc.gnu.org/viewcvs?rev=202418&root=gcc&view=rev
Log:
    PR target/58330
gcc/
    * config/rs6000/rs6000.md (bswapdi2_64bit): Disable for volatile mems.
gcc/testsuite/
    * gcc.target/powerpc/pr58330.c: New.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.md
    trunk/gcc/testsuite/ChangeLog

[Bug target/58330] powerpc64 atomic store split in two

[amodra at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58330

--- Comment #4 from Alan Modra <amodra at gcc dot gnu.org> ---
Author: amodra
Date: Tue Sep 10 08:38:57 2013
New Revision: 202428

URL: http://gcc.gnu.org/viewcvs?rev=202428&root=gcc&view=rev
Log:
PR target/58330
oops, missed from commit


Added:
    trunk/gcc/testsuite/gcc.target/powerpc/pr58330.c

[Bug target/58330] powerpc64 atomic store split in two

[amodra at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58330

--- Comment #5 from Alan Modra <amodra at gcc dot gnu.org> ---
Author: amodra
Date: Mon Sep 23 14:27:06 2013
New Revision: 202829

URL: http://gcc.gnu.org/viewcvs?rev=202829&root=gcc&view=rev
Log:
    PR target/58330
    * config/rs6000/rs6000.md (bswapdi2_64bit): Disable for volatile mems.
gcc/testsuite/
    * gcc.target/powerpc/pr58330.c: New.


Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/rs6000/rs6000.md
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog

[Bug target/58330] powerpc64 atomic store split in two

[amodra at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58330

Alan Modra <amodra at gmail dot com> changed:

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

--- Comment #7 from Alan Modra <amodra at gmail dot com> ---
Fixed

[Bug target/58330] powerpc64 atomic store split in two

[amodra at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58330

--- Comment #6 from Alan Modra <amodra at gcc dot gnu.org> ---
Author: amodra
Date: Mon Sep 23 14:27:56 2013
New Revision: 202830

URL: http://gcc.gnu.org/viewcvs?rev=202830&root=gcc&view=rev
Log:
    PR target/58330
    * gcc.target/powerpc/pr58330.c: New.


Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.target/powerpc/pr58330.c

[Bug target/58330] powerpc64 atomic store split in two

[amodra at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58330

Alan Modra <amodra at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.2