[HACKERS] GCC memory barriers are missing cc clobbers

2014-09-19 Thread Andres Freund
Hi, barrier.h defines memory barriers for x86 as: 32bit: #define pg_memory_barrier() \ __asm__ __volatile__ (lock; addl $0,0(%%esp) : : : memory) 64bit: #define pg_memory_barrier() \ __asm__ __volatile__ (lock; addl $0,0(%%rsp) : : : memory) But addl sets

Re: [HACKERS] GCC memory barriers are missing cc clobbers

2014-09-19 Thread Andres Freund
Hi, On 2014-09-19 12:00:16 +0200, Andres Freund wrote: barrier.h defines memory barriers for x86 as: 32bit: #define pg_memory_barrier() \ __asm__ __volatile__ (lock; addl $0,0(%%esp) : : : memory) 64bit: #define pg_memory_barrier() \ __asm__ __volatile__

Re: [HACKERS] GCC memory barriers are missing cc clobbers

2014-09-19 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: On 2014-09-19 12:00:16 +0200, Andres Freund wrote: But addl sets condition flags. So this really also needs a cc clobber? Or am I missing something? What I missed is that x86 has an implied cc clobber for every inline assembly statement. So forget

Re: [HACKERS] GCC memory barriers are missing cc clobbers

2014-09-19 Thread Andres Freund
On 2014-09-19 09:58:01 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: On 2014-09-19 12:00:16 +0200, Andres Freund wrote: But addl sets condition flags. So this really also needs a cc clobber? Or am I missing something? What I missed is that x86 has an implied cc

Re: [HACKERS] GCC memory barriers are missing cc clobbers

2014-09-19 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes: On 2014-09-19 09:58:01 -0400, Tom Lane wrote: While it might not be buggy as it stands, I think we should add the cc rather than rely on it being implicit. One reason is that people will look at the x86 cases when developing code for other

Re: [HACKERS] GCC memory barriers are missing cc clobbers

2014-09-19 Thread Andres Freund
On 2014-09-19 10:58:56 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: I don't really see any need to backpatch though, do you? Well, I'd make it the same in all branches which have that code, which is not very far back is it? It was already introduced in 9.2 - no idea