[Beignet] [PATCH] Backend: fix one bug of long mad_sat.

[Zhigang Gong]

LGTM, pushed. Thanks.

On Fri, Jan 30, 2015 at 01:22:56PM +0800, junyan.he at inbox.com wrote:
> From: Junyan He <junyan.he at linux.intel.com>
> 
> The bug caused because we didn't consider two overflow
> cases when type is long, which do not cause carry and
> are easy to be ignored.
> 
> V2:
>    Clean some verbose NOPs.
> 
> Signed-off-by: Junyan He <junyan.he at linux.intel.com>
> ---
>  backend/src/backend/gen8_context.cpp | 38 ++++++++++++++++++++++++++++++------
>  utests/compiler_long_hi_sat.cpp      |  8 ++++++--
>  2 files changed, 38 insertions(+), 8 deletions(-)
> 
> diff --git a/backend/src/backend/gen8_context.cpp b/backend/src/backend/gen8_context.cpp
> index cde87de..aa870ea 100644
> --- a/backend/src/backend/gen8_context.cpp
> +++ b/backend/src/backend/gen8_context.cpp
> @@ -332,7 +332,6 @@ namespace gbe
>        sum.type = GEN_TYPE_UL;
>        src2.type = GEN_TYPE_L;
>        dst_l.type = GEN_TYPE_UL;
> -      p->NOP();
>        p->ADD(sum, src2, dst_l);
>  
>        /* Implement this logic:
> @@ -359,7 +358,6 @@ namespace gbe
>        p->MOV(dst_h, GenRegister::immint64(0x7FFFFFFFFFFFFFFFLL));
>        p->MOV(sum, GenRegister::immuint64(0xFFFFFFFFFFFFFFFFULL));
>        p->pop();
> -      p->NOP();
>  
>        /* Implement this logic:
>        else {
> @@ -385,15 +383,19 @@ namespace gbe
>        p->MOV(dst_h, GenRegister::immint64(-0x7FFFFFFFFFFFFFFFLL - 1LL));
>        p->MOV(sum, GenRegister::immud(0));
>        p->pop();
> -      p->NOP();
>  
>        /* saturate logic:
>        if(dst_h > 0)
>          sum = CL_LONG_MAX;
> -      else if(dst_h < -1)
> +      else if (dst_h == 0 && sum > 0x7FFFFFFFFFFFFFFFLL) {
> +        sum = CL_LONG_MAX;
> +      else if (dst_h == -1 && sum < 0x8000000000000000)
> +        sum = CL_LONG_MIN;
> +      else (dst_h < -1)
>          sum = CL_LONG_MIN;
>        cl_long result = (cl_long) sum; */
>        p->MOV(dst_l, sum);
> +      tmp0.type = GEN_TYPE_UL;
>  
>        dst_h.type = GEN_TYPE_L;
>        p->push();
> @@ -405,7 +407,32 @@ namespace gbe
>        p->curr.predicate = GEN_PREDICATE_NORMAL;
>        p->MOV(dst_l, GenRegister::immint64(0x7FFFFFFFFFFFFFFFLL));
>        p->pop();
> -      p->NOP();
> +
> +      p->push();
> +      p->curr.predicate = GEN_PREDICATE_NONE;
> +      p->curr.noMask = 1;
> +      p->curr.useFlag(flagReg.flag_nr(), flagReg.flag_subnr());
> +      p->CMP(GEN_CONDITIONAL_EQ, dst_h, GenRegister::immd(0x0L), tmp1);
> +      p->curr.noMask = 0;
> +      p->curr.predicate = GEN_PREDICATE_NORMAL;
> +      p->MOV(tmp0, GenRegister::immuint64(0x7FFFFFFFFFFFFFFFUL));
> +      p->CMP(GEN_CONDITIONAL_G, dst_l, tmp0, tmp1);
> +      p->MOV(dst_l, GenRegister::immint64(0x7FFFFFFFFFFFFFFFLL));
> +      p->pop();
> +
> +      p->push();
> +      p->curr.predicate = GEN_PREDICATE_NONE;
> +      p->curr.noMask = 1;
> +      p->curr.useFlag(flagReg.flag_nr(), flagReg.flag_subnr());
> +      /* Fixme: HW bug ? 0xFFFFFFFFFFFFFFFF != 0xFFFFFFFFFFFFFFFF */
> +      p->ADD(tmp0, dst_h, GenRegister::immud(1));
> +      p->CMP(GEN_CONDITIONAL_EQ, tmp0, GenRegister::immud(0), tmp1);
> +      p->curr.noMask = 0;
> +      p->curr.predicate = GEN_PREDICATE_NORMAL;
> +      p->MOV(tmp0, GenRegister::immuint64(0x8000000000000000UL));
> +      p->CMP(GEN_CONDITIONAL_L, dst_l, tmp0, tmp1);
> +      p->MOV(dst_l, GenRegister::immint64(-0x7FFFFFFFFFFFFFFFLL - 1LL));
> +      p->pop();
>  
>        p->push();
>        p->curr.predicate = GEN_PREDICATE_NONE;
> @@ -416,7 +443,6 @@ namespace gbe
>        p->curr.predicate = GEN_PREDICATE_NORMAL;
>        p->MOV(dst_l, GenRegister::immint64(-0x7FFFFFFFFFFFFFFFLL - 1LL));
>        p->pop();
> -      p->NOP();
>      }
>    }
>  
> diff --git a/utests/compiler_long_hi_sat.cpp b/utests/compiler_long_hi_sat.cpp
> index e1d5f3b..a0f2bfa 100644
> --- a/utests/compiler_long_hi_sat.cpp
> +++ b/utests/compiler_long_hi_sat.cpp
> @@ -57,7 +57,7 @@ static void __mad_sat(int64_t sourceA, int64_t sourceB, int64_t sourceC, int64_t
>    cl_long multHi;
>    cl_ulong multLo;
>    __64_mul_64(sourceA, sourceB, multLo, multHi);
> -
> +  //printf("hi is %lx, lo is %lx\n", multHi, multLo);
>    cl_ulong sum = multLo + sourceC;
>  
>    // carry if overflow
> @@ -82,6 +82,10 @@ static void __mad_sat(int64_t sourceA, int64_t sourceB, int64_t sourceC, int64_t
>    // saturate
>    if( multHi > 0 )
>      sum = CL_LONG_MAX;
> +  else if ( multHi == 0 && sum > CL_LONG_MAX)
> +    sum = CL_LONG_MAX;
> +  else if ( multHi == -1 && sum < (cl_ulong)CL_LONG_MIN)
> +    sum = CL_LONG_MIN;
>    else if( multHi < -1 )
>      sum = CL_LONG_MIN;
>  
> @@ -161,7 +165,7 @@ void compiler_long_mul_sat(void)
>      uint64_t a = rand();
>      a = a <<32 | a;
>      src[i] = a;
> -//    printf(" 0x%lx", src[i]);
> +    //printf(" 0x%lx", src[i]);
>    }
>  
>    OCL_MAP_BUFFER(0);
> -- 
> 1.9.1
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet