[Beignet] [PATCH] Remove global barrier assert.

[Zhigang Gong]

Nanhai,

I'm not worry about the barrier itself. I'm worry about the memory fence
part. Please refer the following
statement from Open CL spec.

The barrier CLK_LOCAL_MEM_FENCE -
function will either flush any variables stored in
local memory or queue a memory fence to ensure
correct ordering of memory operations to local
memory.

CLK_GLOBAL_MEM_FENCE - The barrier
function will queue a memory fence to ensure
correct ordering of memory operations to global
memory. This can be useful when work-items, for
example, write to buffer or image objects and
then want to read the updated data.

And current implementation does nothing for the global memory operations.

And IMHO, if we can't make sure the implementation comply with OpenCL spec
for the specified memory fence function, we can't just push the code now.
It may cause weird problem in the future which is extremely hard to debug.
Any comments? 

> -----Original Message-----
> From: Zou, Nanhai [mailto:nanhai.zou at intel.com]
> Sent: Friday, June 14, 2013 12:13 PM
> To: Zhigang Gong; Yang, Rong R
> Cc: beignet at lists.freedesktop.org
> Subject: RE: [Beignet] [PATCH] Remove global barrier assert.
> 
> From the spec, it seems that the logic is correct, the barrier is not
limited to
> SHM.
> Let's push the code till we hit bug.
> 
> Thanks
> Zou Nanhai
> 
> -----Original Message-----
> From: beignet-bounces+nanhai.zou=intel.com at lists.freedesktop.org
> [mailto:beignet-bounces+nanhai.zou=intel.com at lists.freedesktop.org] On
> Behalf Of Zhigang Gong
> Sent: Friday, June 14, 2013 11:50 AM
> To: Yang, Rong R
> Cc: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] Remove global barrier assert.
> 
> With this patch, you treat a global memory fence the same as a local
memory
> fence.
> I haven't check the bspec details, but I am doubt it. Need some time to
> investigate the spec.
> 
> On Fri, Jun 14, 2013 at 11:01:09AM +0800, Yang Rong wrote:
> > Per openCL spec, Global memory barrier is consistent across work-items
> > in a single work group, which is match the bspec's barrier. So remove
> > global barrier assert.
> >
> > Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> > ---
> >  backend/src/backend/gen_insn_selection.cpp |    4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/backend/src/backend/gen_insn_selection.cpp
> > b/backend/src/backend/gen_insn_selection.cpp
> > index 88f9e94..3a139ea 100644
> > --- a/backend/src/backend/gen_insn_selection.cpp
> > +++ b/backend/src/backend/gen_insn_selection.cpp
> > @@ -1607,10 +1607,6 @@ namespace gbe
> >      INLINE bool emitOne(Selection::Opaque &sel, const
ir::SyncInstruction
> &insn) const
> >      {
> >        using namespace ir;
> > -      const uint32_t params = insn.getParameters();
> > -      GBE_ASSERTM(params == syncLocalBarrier,
> > -                  "Only barrier(CLK_LOCAL_MEM_FENCE) is supported
> right now "
> > -                  "for the synchronization primitives");
> >        const ir::Register reg = sel.reg(FAMILY_DWORD);
> >
> >        sel.push();
> > --
> > 1.7.10.4
> >
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet