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

Zhigang Gong zhigang.gong at linux.intel.com
Thu Jan 29 20:08:20 PST 2015


The patch LGTM, except the unecessary NOPs.
Please send another version to remove the NOPs and some other NOPs in the same file.

Thanks.

On Thu, Jan 29, 2015 at 07:25:00PM +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.
> 
> Signed-off-by: Junyan He <junyan.he at linux.intel.com>
> ---
>  backend/src/backend/gen8_context.cpp | 35 ++++++++++++++++++++++++++++++++++-
>  utests/compiler_long_hi_sat.cpp      |  8 ++++++--
>  2 files changed, 40 insertions(+), 3 deletions(-)
> 
> diff --git a/backend/src/backend/gen8_context.cpp b/backend/src/backend/gen8_context.cpp
> index cde87de..26bc50d 100644
> --- a/backend/src/backend/gen8_context.cpp
> +++ b/backend/src/backend/gen8_context.cpp
> @@ -390,10 +390,15 @@ namespace gbe
>        /* 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();
> @@ -411,6 +416,34 @@ namespace gbe
>        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->NOP();
> +
> +      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->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_L, dst_h, GenRegister::immd(-1), tmp1);
>        p->curr.noMask = 0;
>        p->curr.predicate = GEN_PREDICATE_NORMAL;
> 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


More information about the Beignet mailing list