[Beignet] [PATCH] Fix untyped write random fail bug.

[Yang, Rong R]

Hi, Yi,

  The random fail that I mention is the untyped write random fail I met. Not the unit test random fail. 

-----Original Message-----
From: Sun, Yi 
Sent: Monday, August 12, 2013 4:45 PM
To: Yang, Rong R; Zhigang Gong
Cc: beignet at lists.freedesktop.org
Subject: RE: [Beignet] [PATCH] Fix untyped write random fail bug.

How many times did you run utest_run, Rong?
The random fail issue is likely still there. There is at least once failure every 20 rounds utest_run running.
for(( i=0; i<20; i++));do echo $i; ./utests/utest_run |grep FAILED; done

Does anyone else reproduce this issue with above command?

Thanks
  --Sun, Yi

> -----Original Message-----
> From: beignet-bounces+yi.sun=intel.com at lists.freedesktop.org
> [mailto:beignet-bounces+yi.sun=intel.com at lists.freedesktop.org] On 
> Behalf Of Yang, Rong R
> Sent: Monday, August 12, 2013 1:26 PM
> To: Zhigang Gong
> Cc: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] Fix untyped write random fail bug.
> 
> Yes, your patch also can fix this random fail. Maybe this is root cause.
> 
> -----Original Message-----
> From: Zhigang Gong [mailto:zhigang.gong at gmail.com]
> Sent: Saturday, August 10, 2013 12:23 AM
> To: Yang, Rong R
> Cc: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] Fix untyped write random fail bug.
> 
> Both surface 0 and 1 should map to the same memory space.
> So it's a little interesting for me, why you change the bti from 1 to 
> 0 can fix the bug. After I look at the surface 0 and 1's 
> initialization, I guess I may find the root cause.
> 
> Can you just apply the following patch:
> 
> diff --git a/src/intel/intel_gpgpu.c b/src/intel/intel_gpgpu.c index
> e553a55..8e90641 100644
> --- a/src/intel/intel_gpgpu.c
> +++ b/src/intel/intel_gpgpu.c
> @@ -478,7 +478,7 @@ intel_gpgpu_map_address_space(intel_gpgpu_t
> *gpgpu)
>    ss1->ss2.width  = ss0->ss2.width  = 127;   /* bits 6:0 of sz */
>    ss1->ss2.height = ss0->ss2.height = 16383; /* bits 20:7 of sz */
>    ss0->ss3.depth  = 1023; /* bits 30:21 of sz */
> -  ss1->ss3.depth  = 510;  /* bits 30:21 of sz */
> +  ss1->ss3.depth  = 1023;  /* bits 30:21 of sz */
>    ss1->ss5.cache_control = ss0->ss5.cache_control = cc_llc_l3;
>    heap->binding_table[0] = offsetof(surface_heap_t, surface);
>    heap->binding_table[1] = sizeof(gen7_surface_state_t) + 
> offsetof(surface_heap_t, surface);
> 
> And don't change the bti, and see whether it can fix the bug you met.
> 
> BTW, the above patch can fix another long standing weird failure of 
> the unit test case: flat_address_space.
> 
> On Fri, Aug 09, 2013 at 04:27:02PM +0800, Yang Rong wrote:
> > When run async copy conformance test, int, uint, float will random fail.
> > It is caused by untyped write's bti, just set the bti same as untyped read.
> >
> > Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> > ---
> >  backend/src/backend/gen_insn_selection.cpp |   10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/backend/src/backend/gen_insn_selection.cpp
> > b/backend/src/backend/gen_insn_selection.cpp
> > index d4be8bf..a21687d 100644
> > --- a/backend/src/backend/gen_insn_selection.cpp
> > +++ b/backend/src/backend/gen_insn_selection.cpp
> > @@ -1957,7 +1957,7 @@ namespace gbe
> >        GenRegister addr;
> >        vector<GenRegister> value(valueNum);
> >
> > -      addr = GenRegister::retype(sel.selReg(insn.getSrc(addrID)),
> GEN_TYPE_F);;
> > +      addr = GenRegister::retype(sel.selReg(insn.getSrc(addrID)),
> > + GEN_TYPE_F);
> >        for (uint32_t valueID = 0; valueID < valueNum; ++valueID)
> >          value[valueID] =
> GenRegister::retype(sel.selReg(insn.getValue(valueID)), GEN_TYPE_F);
> >        sel.UNTYPED_WRITE(addr, value.data(), valueNum, bti); @@
> > -2008,17 +2008,17 @@ namespace gbe
> >      {
> >        using namespace ir;
> >        const AddressSpace space = insn.getAddressSpace();
> > -      const uint32_t bti = space == MEM_LOCAL ? 0xfe : 0x01;
> > +      //const uint32_t bti = space == MEM_LOCAL ? 0xfe : 0x01;
> >        const Type type = insn.getValueType();
> >        const uint32_t elemSize = getByteScatterGatherSize(type);
> >        if (insn.isAligned() == true && elemSize ==
> GEN_BYTE_SCATTER_QWORD)
> > -        this->emitWriteFloat64(sel, insn, bti);
> > +        this->emitWriteFloat64(sel, insn, space == MEM_LOCAL ? 0xfe :
> > + 0x00);
> >        else if (insn.isAligned() == true && elemSize ==
> GEN_BYTE_SCATTER_DWORD)
> > -        this->emitUntypedWrite(sel, insn, bti);
> > +        this->emitUntypedWrite(sel, insn, space == MEM_LOCAL ? 0xfe :
> > + 0x00);
> >        else {
> >          const GenRegister address = sel.selReg(insn.getAddress());
> >          const GenRegister value = sel.selReg(insn.getValue(0));
> > -        this->emitByteScatter(sel, insn, elemSize, address, value, bti);
> > +        this->emitByteScatter(sel, insn, elemSize, address, value, 
> > + space == MEM_LOCAL ? 0xfe : 0x01);
> >        }
> >        return true;
> >      }
> > --
> > 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