[Beignet] [PATCH] GBE: Fix a bug in encoding MATH instruction

Song, Ruiling ruiling.song at intel.com
Mon Jun 24 00:14:56 PDT 2013


Later, I will send some unit tests for byte/short arithmetic. The tests will fail without this patch.
I will send the byte/short support patch together with these unit tests.

-----Original Message-----
From: Zhigang Gong [mailto:zhigang.gong at linux.intel.com] 
Sent: Monday, June 24, 2013 3:05 PM
To: Song, Ruiling
Cc: beignet at lists.freedesktop.org
Subject: Re: [Beignet] [PATCH] GBE: Fix a bug in encoding MATH instruction

Nice catch. I will push it latter.

And, if you have such a unit test case can hit this bug, please also submit it here. Thanks.

On Mon, Jun 24, 2013 at 10:53:57AM +0800, Ruiling Song wrote:
> For std::vector, a push_back may cause memory relocation if no enough 
> memory in the vector pool. And iterator or pointer got previously will 
> become invalid after relocation.
> 
> Here in GenEncoder::next(), which will call push_back(), memory 
> relocation may occur. Then relocation will make 'insn' point to 
> invalid memory that does not belong to GenEncoder::store anymore.
> Then, the setting of execution_width will fail.
> 
> Signed-off-by: Ruiling Song <ruiling.song at intel.com>
> ---
>  backend/src/backend/gen_encoder.cpp |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/backend/src/backend/gen_encoder.cpp 
> b/backend/src/backend/gen_encoder.cpp
> index 3d8afe8..ae981b2 100644
> --- a/backend/src/backend/gen_encoder.cpp
> +++ b/backend/src/backend/gen_encoder.cpp
> @@ -1010,7 +1010,9 @@ namespace gbe
>  
>       if (function == GEN_MATH_FUNCTION_INT_DIV_QUOTIENT ||
>           function == GEN_MATH_FUNCTION_INT_DIV_REMAINDER) {
> -        if(insn->header.execution_size == GEN_WIDTH_16) {
> +        insn->header.execution_size = GEN_WIDTH_8;
> +
> +        if(this->curr.execWidth == 16) {
>            GenInstruction *insn2 = this->next(GEN_OPCODE_MATH);
>            GenRegister new_dest, new_src0, new_src1;
>            new_dest = GenRegister::QnPhysical(dst, 1); @@ -1024,7 
> +1026,6 @@ namespace gbe
>            this->setSrc1(insn2, new_src1);
>          }
>  
> -        insn->header.execution_size = GEN_WIDTH_8;
>       }
>    }
>  
> --
> 1.7.9.5
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list