[Bug target/36793] New: x86-64 does not get __sync_synchronize right

[Bug target/36793] New: x86-64 does not get __sync_synchronize right

[jfc at mit dot edu]

As I understand __sync_synchronize, the intent is to emit a memory barrier
instruction, at least on multiprocessor systems.  Currently on x86
__sync_synchronize inhibits explicit code motion across the builtin function
call but not processor reordering of memory operations across the builtin
function call.

I think mfence is the right instruction on x86-64, and this or a similar
pattern should be added to sync.md:

(define_insn "memory_barrier"
  [(unspec_volatile [(const_int 0)] UNSPEC_MFENCE)]
  "TARGET_SSE2"
  "mfence")


-- 
           Summary: x86-64 does not get __sync_synchronize right
           Product: gcc
           Version: 4.3.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: jfc at mit dot edu
 GCC build triplet: x86_64-linux-gnu
  GCC host triplet: x86_64-linux-gnu
GCC target triplet: x86_64-linux-gnu


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

[Bug target/36793] x86-64 does not get __sync_synchronize right

[samuel dot thibault at ens-lyon dot org]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1103 bytes --]



------- Comment #1 from samuel dot thibault at ens-lyon dot org  2008-11-21 11:16 -------
Just to confirm the bug: the gcc doc says it follows the Intel itanium binary
interface. The Intel documentation says « Associated with each instrinsic are
certain memory barrier properties that restrict the movement of memory
references to visible data across the intrinsic operation (by either the
compiler or the processor). » Not including the mfence instruction would let
the processor move references across the instruction, so it is mandatory.  And
that is not only for x86_64, but also x86, on which you can use e.g. a locked
nop if you don't know the arch, or a mfence when using -march= (IIRC it
appeared with SSE2) 


-- 

samuel dot thibault at ens-lyon dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |samuel dot thibault at ens-
                   |                            |lyon dot org


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

[Bug target/36793] x86-64 does not get __sync_synchronize right

[ubizjak at gmail dot com]

------- Comment #2 from ubizjak at gmail dot com  2008-11-21 17:22 -------
H.J. can probably confirm this.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hjl dot tools at gmail dot
                   |                            |com, ubizjak at gmail dot
                   |                            |com


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

[Bug target/36793] x86-64 does not get __sync_synchronize right

[hjl dot tools at gmail dot com]

------- Comment #3 from hjl dot tools at gmail dot com  2008-11-21 17:36 -------
__sync_synchronize isn't specified for IA32/Intel64. You can check
out Intel Memory Ordering White Paper:

www.intel.com/products/processor/manuals/318147.pdf

to see what is the most appropriate.


-- 


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

[Bug target/36793] x86-64 does not get __sync_synchronize right

[hjl dot tools at gmail dot com]

------- Comment #4 from hjl dot tools at gmail dot com  2008-11-21 17:37 -------
The Intel Memory Ordering White Paper is at

http://www.intel.com/products/processor/manuals/318147.pdf


-- 


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

[Bug target/36793] x86-64 does not get __sync_synchronize right

[samuel dot thibault at ens-lyon dot org]

------- Comment #5 from samuel dot thibault at ens-lyon dot org  2008-11-21 23:20 -------
We do already know which x86 memory barrier instruction we need, that's not the
problem, no need to give us pointers to documentations. The problem is that
we'd like to not use explicit x86 instructions but rather rely on the
"portability" of gcc's __sync_synchronize.  If your answer is "sync_foobar is
not specified for baz", all these __sync_foobar become useless, since the user
can't assume what their are supposed to achieve.  From what the documentation
says, I had assumed that gcc would implement for other archs what Intel
documents for ia64, but it happens not to be the case at least for
sync_synchronize.  So in short, if that's your answer to the bug, then I'll
have to tell people _not_ to use gcc's __sync_* at all except on ia64, since
that would be the only arch on which we would have any semantic guarantee...
(and then the documentation needs to be fixed).  Of course, that's not an
option I'd like, and actually I believe there may be SMP bugs in libgc & such
that use it... 


-- 


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

[Bug target/36793] x86-64 does not get __sync_synchronize right

[hjl dot tools at gmail dot com]

------- Comment #6 from hjl dot tools at gmail dot com  2008-11-21 23:38 -------
I think it is a bug.


-- 


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

[Bug target/36793] x86-64 does not get __sync_synchronize right

[ubizjak at gmail dot com]

------- Comment #7 from ubizjak at gmail dot com  2008-11-22 18:29 -------
Patch that implements "memory_barrier" for x86 at [1].

[1] http://gcc.gnu.org/ml/gcc-patches/2008-11/msg01181.html


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |ubizjak at gmail dot com
                   |dot org                     |
                URL|                            |http://gcc.gnu.org/ml/gcc-
                   |                            |patches/2008-
                   |                            |11/msg01181.html
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2008-11-22 18:29:20
               date|                            |


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

[Bug target/36793] x86-64 does not get __sync_synchronize right

[samuel dot thibault at ens-lyon dot org]

------- Comment #8 from samuel dot thibault at ens-lyon dot org  2008-11-22 19:41 -------
Ah, well, by "nop", I was thinking about things like what Linux does: lock;
addl $0,0(%%esp) 


-- 


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

[Bug target/36793] x86-64 does not get __sync_synchronize right

[uros at gcc dot gnu dot org]

------- Comment #9 from uros at gcc dot gnu dot org  2008-11-24 16:57 -------
Subject: Bug 36793

Author: uros
Date: Mon Nov 24 16:55:49 2008
New Revision: 142160

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=142160
Log:
        * config/i386/i386.md (UNSPECV_CMPXCHG): Rename from
        UNSPECV_CMPXCHG_[12].
        * config/i386/sync.md: Use UNSPECV_CMPXCHG instead of
        UNSPECV_CMPXCHG_[12].

        PR target/36793
        * config/i386/sync.md (memory_barrier): New expander.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.md
    trunk/gcc/config/i386/sync.md


-- 


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

[Bug target/36793] x86-64 does not get __sync_synchronize right

[ubizjak at gmail dot com]

------- Comment #10 from ubizjak at gmail dot com  2008-11-24 16:59 -------
Fixed.


-- 

ubizjak at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.4.0


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

[Bug target/36793] x86-64 does not get __sync_synchronize right

[ubizjak at gmail dot com]

------- Comment #11 from ubizjak at gmail dot com  2008-11-25 09:15 -------
Should we fix __sync_synchronize in 4.3 too?


-- 


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