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, [email protected] wrote: > From: Junyan He <[email protected]> > > 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 <[email protected]> > --- > 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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
