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

[Zhigang Gong]

After some discussion, the original patch is to optimize the second
JMPI which has lower probability. I changed it to optimize the
first JMPI which has higher probability and could optimize better.

Just pushed the new version patch. Thanks.

On Thu, May 08, 2014 at 11:05:36AM +0800, Zhigang Gong wrote:
> 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
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet