[Beignet] [PATCH] GBE: fixed a prediction bug in typed write instruction.

[Zhigang Gong]

Thanks for testing. And I just pushed those patches with your tested-by tag.

> -----Original Message-----
> From:
> beignet-bounces+zhigang.gong=linux.intel.com at lists.freedesktop.org
> [mailto:beignet-bounces+zhigang.gong=linux.intel.com at lists.freedesktop.
> org] On Behalf Of Lv, Meng
> Sent: Wednesday, May 15, 2013 4:41 PM
> To: Zhigang Gong
> Cc: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] GBE: fixed a prediction bug in typed write
> instruction.
> 
> The patch have fixed the write_imagef bug, and all utest case passed.
> Thanks.
> 
> -----Original Message-----
> From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com]
> Sent: Wednesday, May 15, 2013 2:48 PM
> To: Lv, Meng
> Cc: beignet at lists.freedesktop.org
> Subject: RE: [Beignet] [PATCH] GBE: fixed a prediction bug in typed write
> instruction.
> 
> Hi Lv Meng,
> 
> I finally identified the bug in write_image function, and this patch is to
fix it.
> It is based on my prveious sampler/image v2 patch. Please apply those
> patchses at first, then apply this one and try it with you program.
> Please tell me the result. Thanks.
> 
> 
> > -----Original Message-----
> > From:
> > beignet-bounces+zhigang.gong=linux.intel.com at lists.freedesktop.org
> >
> [mailto:beignet-bounces+zhigang.gong=linux.intel.com at lists.freedesktop.
> > org] On Behalf Of Zhigang Gong
> > Sent: Wednesday, May 15, 2013 12:48 PM
> > To: beignet at lists.freedesktop.org
> > Cc: Zhigang Gong
> > Subject: [Beignet] [PATCH] GBE: fixed a prediction bug in typed write
> > instruction.
> >
> > We need to put the header initialization and the LOD initialization to
> > no mask and no predication state. For all the other parts, we need to
> > enable mask and use the current predication state and need to set the
> > quater control properly. Otherwise, when write_image is called in a
> > condition-branch block, it will trigger this bug and doesn't write
> > data
> out
> > correctly.
> >
> > Signed-off-by: Zhigang Gong <zhigang.gong at linux.intel.com>
> > ---
> >  backend/src/backend/gen_context.cpp | 16 ++++++++++------
> > backend/src/backend/gen_encoder.cpp |  2 --
> >  2 files changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/backend/src/backend/gen_context.cpp
> > b/backend/src/backend/gen_context.cpp
> > index cacc3e3..9158a33 100644
> > --- a/backend/src/backend/gen_context.cpp
> > +++ b/backend/src/backend/gen_context.cpp
> > @@ -303,7 +303,6 @@ namespace gbe
> >      if (insn.src(8).reg() != 0)
> >        p->MOV(GenRegister::f8grf(nr + (simdWidth/4), 0), wcoord);
> >      p->SAMPLE(dst, msgPayload, false, bti, sampler, simdWidth, -1,
> > 0);
> > -
> >      p->pop();
> >    }
> >
> > @@ -329,13 +328,17 @@ namespace gbe
> >      // prepare mesg desc and move to a0.0.
> >      // desc = bti | (msg_type << 14) | (header_present << 19))
> >      // prepare header, we need to enable all the 8 planes.
> > -    p->MOV(GenRegister::ud8grf(nr, 7), GenRegister::immud(0xff));
> > -    // Typed write only support SIMD8.
> > +    p->MOV(GenRegister::ud8grf(nr, 7), GenRegister::immud(0xffff));
> >      p->curr.execWidth = 8;
> > +    // Typed write only support SIMD8.
> >      // Prepare message payload U + V + R(ignored) + LOD(0) + RGBA.
> > -    // XXX currently only support U32 surface type with RGBA.
> > +    // Currently, we don't support non-zero lod, so we clear all lod to
> > +    // zero for both quarters thus save one instruction here.
> > +    // Thus we must put this instruction in noMask and no predication
> > state.
> >      p->MOV(GenRegister::ud8grf(nr + 4, 0), GenRegister::immud(0));
> > //LOD
> > -
> > +    p->pop();
> > +    p->push();
> > +    p->curr.execWidth = 8;
> >      // TYPED WRITE send instruction only support SIMD8, if we are
> > SIMD16, we
> >      // need to call it twice.
> >      uint32_t quarterNum = (simdWidth == 8) ? 1 : 2; @@ -346,6
> +349,8
> > @@ namespace gbe
> >
> > GenRegister::retype(GenRegister::QnPhysical(src, quarter), src.type))
> > #define QUARTER_MOV1(dst_nr, src)
> > p->MOV(GenRegister::retype(GenRegister::ud8grf(dst_nr, 0), src.type),
> > p->\
> >
> > GenRegister::retype(GenRegister::QnPhysical(src,quarter), src.type))
> > +      if (quarter == 1)
> > +        p->curr.quarterControl = GEN_COMPRESSION_Q2;
> >        QUARTER_MOV0(nr + 1, ucoord);
> >        QUARTER_MOV0(nr + 2, vcoord);
> >        if (insn.src(3 + insn.extra.elem).reg() != 0) @@ -357,7 +362,6
> > @@ namespace gbe  #undef QUARTER_MOV
> >        p->TYPED_WRITE(header, true, bti);
> >      }
> > -
> >      p->pop();
> >    }
> >
> > diff --git a/backend/src/backend/gen_encoder.cpp
> > b/backend/src/backend/gen_encoder.cpp
> > index 4688e2c..d6c34fb 100644
> > --- a/backend/src/backend/gen_encoder.cpp
> > +++ b/backend/src/backend/gen_encoder.cpp
> > @@ -851,7 +851,6 @@ namespace gbe
> >       uint32_t simd_mode = (simdWidth == 16) ?
> >                              GEN_SAMPLER_SIMD_MODE_SIMD16 :
> > GEN_SAMPLER_SIMD_MODE_SIMD8;
> >       GenInstruction *insn = this->next(GEN_OPCODE_SEND);
> > -     insn->header.predicate_control = 0; /* XXX */
> >       this->setHeader(insn);
> >       this->setDst(insn, dest);
> >       this->setSrc0(insn, msg);
> > @@ -866,7 +865,6 @@ namespace gbe
> >       GenInstruction *insn = this->next(GEN_OPCODE_SEND);
> >       uint32_t msg_type = GEN_TYPED_WRITE;
> >       uint32_t msg_length = header_present ? 9 : 8;
> > -     insn->header.predicate_control = 0; /* XXX */
> >       this->setHeader(insn);
> >       this->setDst(insn, GenRegister::retype(GenRegister::null(),
> > GEN_TYPE_UD));
> >       this->setSrc0(insn, msg);
> > --
> > 1.7.11.7
> >
> > _______________________________________________
> > 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