Re: [Patch 2/4] ARM 64 bit sync atomic operations [V2]
On 09/30/2011 08:54 PM, Joseph S. Myers wrote: On Fri, 30 Sep 2011, Ramana Radhakrishnan wrote: On 26 July 2011 10:01, Dr. David Alan Gilbert david.gilb...@linaro.org wrote: + +extern unsigned int __write(int fd, const void *buf, unsigned int count); Why are we using __write instead of write? Because plain write is in the user's namespace in ISO C. See what I said in http://gcc.gnu.org/ml/gcc-patches/2011-07/msg00084.html - the alternative is hardcoding the syscall number and using the syscall directly. That would be better, no? Unless __write is part of the glibc API, which AFAIK it isn't. Andrew.
Re: [Patch 2/4] ARM 64 bit sync atomic operations [V2]
On 30 September 2011 18:01, H.J. Lu hjl.to...@gmail.com wrote:
You may want to look a look at:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50583
ARM may have the same problem.
OK - although to be honest this patch only stretches the same
structures to 64bit - any major changes in semantics are a separate issue - but
thanks for pointing it out.
Hmm - I think what's produced is correct; however the manual
description is inconsistent:
These builtins perform the operation suggested by the name, and
returns the value that had previously been in memory. That is,
{ tmp = *ptr; *ptr OP= value; return tmp; }
The ARM code (see below) does a single load inside a loop with a guarded
store. This guarantees that the value returned is the value that
was 'previously been in memory' directly prior to the atomic operation - however
that does mean it doesn't do the pair of accesses implied by the 'tmp
= *ptr; *ptr OP= value'
On ARM the operation for fetch_and_add we get:
(This is pre-my-patch and 32bit, my patch doesn't change the structure
except for the position of that last label):
mov r3, r0
dmb sy
.LSYT6:
ldrex r0, [r3]
add r2, r0, r1
strex r0, r2, [r3]
teq r0, #0
bne .LSYT6
sub r0, r2, r1
dmb sy
That seems the correct semantics to me - if not what am I missing? Was
the intention of the example
really to cause two loads - if so why?
for sync_and_fetch we get:
dmb sy
.LSYT6:
ldrex r0, [r3]
add r0, r0, r1
strex r2, r0, [r3]
teq r2, #0
bne .LSYT6
dmb sy
i.e. the value returned is always the value that goes into the guarded
store - and is hence
always the value that's stored.
Dave
Re: [Patch 2/4] ARM 64 bit sync atomic operations [V2]
On 3 October 2011 09:35, Andrew Haley a...@redhat.com wrote: On 09/30/2011 08:54 PM, Joseph S. Myers wrote: On Fri, 30 Sep 2011, Ramana Radhakrishnan wrote: On 26 July 2011 10:01, Dr. David Alan Gilbert david.gilb...@linaro.org wrote: + +extern unsigned int __write(int fd, const void *buf, unsigned int count); Why are we using __write instead of write? Because plain write is in the user's namespace in ISO C. See what I said in http://gcc.gnu.org/ml/gcc-patches/2011-07/msg00084.html - the alternative is hardcoding the syscall number and using the syscall directly. That would be better, no? Unless __write is part of the glibc API, which AFAIK it isn't. I could change it to calling the syscall directly - although it gets a little messy having to deal with both ARM and Thumb syscalls; I was trying to avoid further complicating an already complicated corner case. Dave
Re: [Patch 2/4] ARM 64 bit sync atomic operations [V2]
On Mon, 3 Oct 2011, Andrew Haley wrote: On 09/30/2011 08:54 PM, Joseph S. Myers wrote: On Fri, 30 Sep 2011, Ramana Radhakrishnan wrote: On 26 July 2011 10:01, Dr. David Alan Gilbert david.gilb...@linaro.org wrote: + +extern unsigned int __write(int fd, const void *buf, unsigned int count); Why are we using __write instead of write? Because plain write is in the user's namespace in ISO C. See what I said in http://gcc.gnu.org/ml/gcc-patches/2011-07/msg00084.html - the alternative is hardcoding the syscall number and using the syscall directly. That would be better, no? Unless __write is part of the glibc API, which AFAIK it isn't. It's exported at version GLIBC_2.0 (not GLIBC_PRIVATE) under the comment functions used in inline functions or macros, although I don't actually see any such functions or macros in current glibc headers. I think being under a public version means you can rely on it staying there. -- Joseph S. Myers jos...@codesourcery.com
Re: [Patch 2/4] ARM 64 bit sync atomic operations [V2]
On Mon, Oct 03, 2011 at 03:55:58PM +, Joseph S. Myers wrote: That would be better, no? Unless __write is part of the glibc API, which AFAIK it isn't. It's exported at version GLIBC_2.0 (not GLIBC_PRIVATE) under the comment functions used in inline functions or macros, although I don't actually see any such functions or macros in current glibc headers. I think being under a public version means you can rely on it staying there. On the architectures where it has been exported, yes. New architectures might prune those. But we are talking here about a particular architecture which had these exported, therefore they will continue to be exported in the future as well, unless glibc SONAME changes (very unlikely). Jakub
Re: [Patch 2/4] ARM 64 bit sync atomic operations [V2]
On 26 July 2011 10:01, Dr. David Alan Gilbert david.gilb...@linaro.org wrote: + +extern unsigned int __write(int fd, const void *buf, unsigned int count); Why are we using __write instead of write? A comment elaborating that this file should only be in the static libgcc and never in the dynamic libgcc would be useful, given that the constructor is only pulled in only if a 64 bit sync primitive is referred to. cheers Ramana
Re: [Patch 2/4] ARM 64 bit sync atomic operations [V2]
On Fri, Sep 30, 2011 at 9:45 AM, Ramana Radhakrishnan ramana.radhakrish...@linaro.org wrote: On 26 July 2011 10:01, Dr. David Alan Gilbert david.gilb...@linaro.org wrote: + +extern unsigned int __write(int fd, const void *buf, unsigned int count); Why are we using __write instead of write? A comment elaborating that this file should only be in the static libgcc and never in the dynamic libgcc would be useful, given that the constructor is only pulled in only if a 64 bit sync primitive is referred to. You may want to look a look at: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50583 ARM may have the same problem. -- H.J.
Re: [Patch 2/4] ARM 64 bit sync atomic operations [V2]
On Fri, 30 Sep 2011, Ramana Radhakrishnan wrote: On 26 July 2011 10:01, Dr. David Alan Gilbert david.gilb...@linaro.org wrote: + +extern unsigned int __write(int fd, const void *buf, unsigned int count); Why are we using __write instead of write? Because plain write is in the user's namespace in ISO C. See what I said in http://gcc.gnu.org/ml/gcc-patches/2011-07/msg00084.html - the alternative is hardcoding the syscall number and using the syscall directly. -- Joseph S. Myers jos...@codesourcery.com
