[Bug target/42869] GOMP_critical_start wrong on Itanium due to __sync miscompilation

[sje at gcc dot gnu dot org]

--- Comment #7 from sje at gcc dot gnu dot org  2010-07-21 22:38 ---
Subject: Bug 42869

Author: sje
Date: Wed Jul 21 22:37:53 2010
New Revision: 162387

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=162387
Log:
2010-07-21  Steve Ellcey  

PR target/42869
* config/ia64/sync.md (sync_compare_and_swap): Move memory fence.

Modified:
branches/gcc-4_4-branch/gcc/ChangeLog
branches/gcc-4_4-branch/gcc/config/ia64/sync.md


-- 


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

[Bug target/42869] GOMP_critical_start wrong on Itanium due to __sync miscompilation

[sje at cup dot hp dot com]

--- Comment #6 from sje at cup dot hp dot com  2010-03-12 18:22 ---
Fixed by moving the mf after the cmpxch.rel.  The cmpxchg.rel will keep memory
operations from moving down and the mf will keep them from moving up.  Changing
cmpxchg.rel to cmpxchg.acq would have achieved the same result.


-- 

sje at cup dot hp dot com changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution||FIXED


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

[Bug target/42869] GOMP_critical_start wrong on Itanium due to __sync miscompilation

[sje at gcc dot gnu dot org]

--- Comment #5 from sje at gcc dot gnu dot org  2010-03-12 18:19 ---
Subject: Bug 42869

Author: sje
Date: Fri Mar 12 18:19:14 2010
New Revision: 157410

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=157410
Log:
2010-03-12  Steve Ellcey  

PR target/42869
* config/ia64/sync.md (sync_compare_and_swap): Move memory fence.

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


-- 


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

[Bug target/42869] GOMP_critical_start wrong on Itanium due to __sync miscompilation

[sje at cup dot hp dot com]

--- Comment #4 from sje at cup dot hp dot com  2010-03-09 23:49 ---
Yes, I think this is clearly a bug in the IA64 definition of
sync_compare_and_swap.  I think the fix is swapping the two instructions being
generated by the IA64 sync_compare_and_swap instruction.  (cmpxchg4.rel
followed by a memory fence instead of a memory fence followed by cmpxchg4.rel).


-- 


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

[Bug target/42869] GOMP_critical_start wrong on Itanium due to __sync miscompilation

[jakub at gcc dot gnu dot org]

--- Comment #3 from jakub at gcc dot gnu dot org  2010-03-09 23:33 ---
__sync_bool_compare_and_swap is documented to be a full barrier, so I think the
bug must be on the ia64/sync.md side.


-- 


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

[Bug target/42869] GOMP_critical_start wrong on Itanium due to __sync miscompilation

[sje at cup dot hp dot com]

--- Comment #2 from sje at cup dot hp dot com  2010-03-09 23:25 ---
I think the code is wrong.  Looking at ToT, config/ia64/sync.md will generate
the code shown where the memory fence is in front of the cmpxchg4.rel
instruction.

cmpxchg4.rel has release semantics which are defined in the Itanium manual as:
"Release instructions guarentee that all previous orderable instructions are
made visible prior to being made visible themselves."  This would imply to me
that the memory fence before the cmpxchg4.rel is not needed but we do need one
after it in order to prevent things being moved up ahead of the cmpxchg4.rel. 
Alternatively, we could change to code to use cmpxchg4.acq instead of
cmpxchg4.rel and keep the memory fence where it is.  cmpxchg4.acq has acquire
semantics instead of release semantics.


-- 


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

[Bug target/42869] GOMP_critical_start wrong on Itanium due to __sync miscompilation

[rguenth at gcc dot gnu dot org]

--- Comment #1 from rguenth at gcc dot gnu dot org  2010-01-26 12:06 ---
Please check trunk.


-- 

rguenth at gcc dot gnu dot org changed:

   What|Removed |Added

  Component|c   |target
 GCC target triplet||ia64-*-*
   Keywords||wrong-code


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