[GHC] #8219: x86 definition of cas() is wrong
#8219: x86 definition of cas() is wrong ------------------------------------+------------------------------------- Reporter: parcs | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Runtime System | Version: 7.7 Keywords: | Operating System: Unknown/Multiple Architecture: Unknown/Multiple | Type of failure: None/Unknown Difficulty: Unknown | Test Case: Blocked By: | Blocking: Related Tickets: | ------------------------------------+------------------------------------- On x86, `cas()` is defined in SMP.h as (paraphrasing) {{{ #!c asm ("lock\ncmpxchg %3,%1" :"=a"(o), "=m" (*p) :"0"(o), "r"(n)); }}} But *p is both read and written to, and therefore should have the '+' constraint modifier as opposed to the '=' modifier. Otherwise, the compiler is allowed to elide an earlier write to *p because it thinks that the call to `cas()` will overwrite it. -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/8219> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#8219: x86 definition of cas() is wrong -------------------------------------+------------------------------------ Reporter: parcs | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Runtime System | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by ezyang): Seems right. Do you have a test-case? -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/8219#comment:1> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#8219: x86 definition of cas() is wrong -------------------------------------+------------------------------------ Reporter: parcs | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Runtime System | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by parcs): Test case: === cas.c {{{ #!c static int cas (int *p, int o, int n) { asm ("lock cmpxchg %2,%1" :"=a"(o), "=m"(*(volatile int *)p) :"0"(o), "r"(n)); return o; } int v; int main (int argc, char **argv) { /* The following assignment gets removed unless you change "=m" to "+m". */ v = 42; cas (&v, 0, 1); return 0; } return 0; }}} === Command Line {{{ $ gcc -O2 -S cas.c $ grep 42 cas.s zsh: exit 1 $ perl -i -pe 's/=m/+m/' cas.c $ gcc -O2 -S cas.c $ grep 42 cas.s movl $42, v(%rip) }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/8219#comment:2> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#8219: x86 definition of cas() is wrong -------------------------------------+------------------------------------ Reporter: parcs | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Runtime System | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by parcs): Sorry, my original test case is needlessly convoluted. Here's a more straightforward one: === cas.c {{{ #!c #include <assert.h> static int cas (int *p, int o, int n) { asm ("lock cmpxchg %3,%1" :"=a"(o), "=m"(*(volatile int *)p) :"0"(o), "r"(n)); return o; } int v; int main (int argc, char **argv) { v = 42; cas (&v, 42, 1); assert(v == 1); return 0; } }}} === Command Line {{{ $ gcc -O2 cas.c $ ./a.out a.out: cas.c:19: main: Assertion `v == 1' failed. }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/8219#comment:3> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#8219: x86 definition of cas() is wrong -------------------------------------+------------------------------------ Reporter: parcs | Owner: Type: bug | Status: new Priority: normal | Milestone: Component: Runtime System | Version: 7.7 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Comment (by Patrick Palka <patrick@…>): In [changeset:84dff71075b915938e2457f5bff2d127eac3b3cc/ghc]: {{{ #!CommitTicketReference repository="ghc" revision="84dff71075b915938e2457f5bff2d127eac3b3cc" Fix the definition of cas() on x86 (#8219) *p is both read and written to by the cmpxchg instruction, and therefore should be given the '+' constraint modifier. (In GCC's extended ASM language, '+' means that the operand is both read and written to whereas '=' means that it is only written to.) Otherwise, the compiler is allowed to rewrite something like SpinLock lock; initSpinLock(&lock); /* sets lock = 1 */ ACQUIRE_SPIN_LOCK(&lock); into SpinLock lock; ACQUIRE_SPIN_LOCK(&lock); because according to the asm statement, the previous value of 'lock' is not important. }}} -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/8219#comment:4> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
#8219: x86 definition of cas() is wrong -------------------------------------+------------------------------------ Reporter: parcs | Owner: Type: bug | Status: closed Priority: normal | Milestone: Component: Runtime System | Version: 7.7 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Unknown/Multiple Type of failure: None/Unknown | Difficulty: Unknown Test Case: | Blocked By: Blocking: | Related Tickets: -------------------------------------+------------------------------------ Changes (by parcs): * status: new => closed * resolution: => fixed -- Ticket URL: <http://ghc.haskell.org/trac/ghc/ticket/8219#comment:5> GHC <http://www.haskell.org/ghc/> The Glasgow Haskell Compiler
participants (1)
-
GHC