Re: [Mesa-dev] [PATCH] gallium/os: add os_wait_until_zero

2015-06-26 Thread Jose Fonseca
As others pointed, volatile and atomic are slightly different things, but you have point: atomic operations should probably take volatile pointers as arguments. This is what C11 did http://en.cppreference.com/w/c/atomic/atomic_load so I do believe that it makes sense to update p_atomic help

Re: [Mesa-dev] [PATCH] gallium/os: add os_wait_until_zero

2015-06-26 Thread Patrick Baggett
On Fri, Jun 26, 2015 at 11:40 AM, Marek Olšák wrote: > If p_atomic_read is fine, then this patch is fine too. So you're > telling that this should work: > > while (p_atomic_read(var)); > > I wouldn't be concerned about a memory barrier. This is only 1 int, so > it should make its way into the sha

Re: [Mesa-dev] [PATCH] gallium/os: add os_wait_until_zero

2015-06-26 Thread Ilia Mirkin
On Fri, Jun 26, 2015 at 12:40 PM, Marek Olšák wrote: > If p_atomic_read is fine, then this patch is fine too. So you're > telling that this should work: > > while (p_atomic_read(var)); No, this shouldn't work. I don't believe that anyone ever claimed it ought to. But perhaps we have different ide

Re: [Mesa-dev] [PATCH] gallium/os: add os_wait_until_zero

2015-06-26 Thread Marek Olšák
If p_atomic_read is fine, then this patch is fine too. So you're telling that this should work: while (p_atomic_read(var)); I wouldn't be concerned about a memory barrier. This is only 1 int, so it should make its way into the shared cache eventually. Marek On Fri, Jun 26, 2015 at 6:25 PM, Ili

Re: [Mesa-dev] [PATCH] gallium/os: add os_wait_until_zero

2015-06-26 Thread Ilia Mirkin
p_atomic_read is fine as-is. The read is guaranteed to be atomic up to a word size on x86 processors. I suspect other cpu's have similar guarantees, and if not, then hopefully have other ops to perform the read atomically. On Fri, Jun 26, 2015 at 12:18 PM, Marek Olšák wrote: > My question was how

Re: [Mesa-dev] [PATCH] gallium/os: add os_wait_until_zero

2015-06-26 Thread Marek Olšák
My question was how to fix p_atomic_read? Would the volatile read that I proposed work? Marek On Fri, Jun 26, 2015 at 5:59 PM, Ilia Mirkin wrote: > The compiler doesn't know that there's another thread. And it won't > start to assume that there might always be another thread because then > it c

Re: [Mesa-dev] [PATCH] gallium/os: add os_wait_until_zero

2015-06-26 Thread Ilia Mirkin
The compiler doesn't know that there's another thread. And it won't start to assume that there might always be another thread because then it could never optimize pointer derefs. On Fri, Jun 26, 2015 at 11:55 AM, Marek Olšák wrote: > I assumed the atomic operation in another thread would act as a

Re: [Mesa-dev] [PATCH] gallium/os: add os_wait_until_zero

2015-06-26 Thread Marek Olšák
I assumed the atomic operation in another thread would act as a barrier in this case. Is that right? Marek On Fri, Jun 26, 2015 at 5:47 PM, Ilia Mirkin wrote: > On Fri, Jun 26, 2015 at 11:33 AM, Marek Olšák wrote: >> I expect the variable will be changed using an atomic operation by the >> CPU,

Re: [Mesa-dev] [PATCH] gallium/os: add os_wait_until_zero

2015-06-26 Thread Ilia Mirkin
On Fri, Jun 26, 2015 at 11:33 AM, Marek Olšák wrote: > I expect the variable will be changed using an atomic operation by the > CPU, or using a coherent store instruction by the GPU. > > If this is wrong and volatile is really required here, then > p_atomic_read is wrong too. Should we fix it? For

Re: [Mesa-dev] [PATCH] gallium/os: add os_wait_until_zero

2015-06-26 Thread Marek Olšák
I expect the variable will be changed using an atomic operation by the CPU, or using a coherent store instruction by the GPU. If this is wrong and volatile is really required here, then p_atomic_read is wrong too. Should we fix it? For example: #define p_atomic_read(_v) (*(volatile int*)(_v)) Th

Re: [Mesa-dev] [PATCH] gallium/os: add os_wait_until_zero

2015-06-26 Thread Ilia Mirkin
On Fri, Jun 26, 2015 at 7:05 AM, Marek Olšák wrote: > From: Marek Olšák > > This will be used by radeon and amdgpu winsyses. > Copied from the amdgpu winsys. > --- > src/gallium/auxiliary/os/os_time.c | 36 +++- > src/gallium/auxiliary/os/os_time.h | 10 ++

Re: [Mesa-dev] [PATCH] gallium/os: add os_wait_until_zero

2015-06-26 Thread Jose Fonseca
On 26/06/15 12:05, Marek Olšák wrote: From: Marek Olšák This will be used by radeon and amdgpu winsyses. Copied from the amdgpu winsys. --- src/gallium/auxiliary/os/os_time.c | 36 +++- src/gallium/auxiliary/os/os_time.h | 10 ++ 2 files changed, 45 i

[Mesa-dev] [PATCH] gallium/os: add os_wait_until_zero

2015-06-26 Thread Marek Olšák
From: Marek Olšák This will be used by radeon and amdgpu winsyses. Copied from the amdgpu winsys. --- src/gallium/auxiliary/os/os_time.c | 36 +++- src/gallium/auxiliary/os/os_time.h | 10 ++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/s