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

[Zhigang Gong]

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), \
> 
> 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