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:[email protected]] On Behalf Of > > > Zhigang Gong > > > Sent: Tuesday, May 06, 2014 12:48 PM > > > To: [email protected] > > > 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 <[email protected]> > > > --- > > > 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 > > > [email protected] > > > http://lists.freedesktop.org/mailman/listinfo/beignet > > > _______________________________________________ > > > Beignet mailing list > > > [email protected] > > > http://lists.freedesktop.org/mailman/listinfo/beignet > > _______________________________________________ > > Beignet mailing list > > [email protected] > > http://lists.freedesktop.org/mailman/listinfo/beignet > _______________________________________________ > Beignet mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/beignet _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
