[Beignet] [PATCH] GBE: fix one potential bug in UnsignedI64ToFloat.

[Zhigang Gong]

On Thu, May 08, 2014 at 10:55:54AM +0800, Zhigang Gong wrote:
> On Thu, May 08, 2014 at 03:28:34AM +0000, Yang, Rong R wrote:
> > 
> > 
> > -----Original Message-----
> > From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of Zhigang Gong
> > Sent: Tuesday, May 06, 2014 12:48 PM
> > To: beignet at lists.freedesktop.org
> > Cc: Gong, Zhigang
> > Subject: [Beignet] [PATCH] GBE: fix one potential bug in UnsignedI64ToFloat.
> > 
> > Set exp to a proper value to make sure all the inactive lanes flag bits are 1s which satisfy the requirement of the following ALL16/ALL8H condition check.
> > 
> > Signed-off-by: Zhigang Gong <zhigang.gong at intel.com>
> > ---
> >  backend/src/backend/gen_context.cpp | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/backend/src/backend/gen_context.cpp b/backend/src/backend/gen_context.cpp
> > index 00cbf1d..f014295 100644
> > --- a/backend/src/backend/gen_context.cpp
> > +++ b/backend/src/backend/gen_context.cpp
> > @@ -907,6 +907,10 @@ namespace gbe
> >                                              GenRegister mantissa, GenRegister tmp, GenRegister flag) {
> >      uint32_t jip0, jip1;
> >      GenRegister dst_ud = GenRegister::retype(dst, GEN_TYPE_UD);
> > +    p->push();
> > +      p->curr.noMask = 1;
> > +      p->MOV(exp, GenRegister::immud(24)); // make sure the inactive lane is 1 when check ALL8H/ALL16H condition latter.
> > >>>>>>Why move 24 to exp. It should initialize to -1 for inactive lane. When all exp is -1 here, ALL8H/ALL16H can jump out the function. So ALL8H/ALL16H is a optimization, and inactive lane should stop this jump.
> 
>       p->CMP(GEN_CONDITIONAL_G, exp, GenRegister::immud(23));
>       p->curr.predicate = GEN_PREDICATE_NORMAL;
>       p->CMP(GEN_CONDITIONAL_L, exp, GenRegister::immud(32));  //exp>23 && high!=0
>       p->ADD(tmp, exp, GenRegister::immud(-23));
>       ...
>       p->curr.predicate = GEN_PREDICATE_ALIGN1_ALL8H/ALL16H;
>       p->JMPI(GenRegister::immud(0));
> 
> The algorithm will check whether all channels are between (23, 32).
> If it is, then it jump some instructions.
> 
> I initialize all channels to 24, so that all the inactive lanes cound satisfy the above checking.
> And if all the active lanes are also within the range, then we can really jump over those instructions.
> 
> Is there anything wrong?
Just checked the code again, found there are two JMPIs. And I missed the first JMPIs.
will send a new version of patch. Thanks.


> > 
> > 
> > +    p->pop();
> >      p->FBH(exp, high);
> >      p->ADD(exp, GenRegister::negate(exp), GenRegister::immud(31));  //exp = 32 when high == 0
> >      p->push();
> > --
> > 1.8.3.2
> > 
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet
> > _______________________________________________
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet