Thanks your comments. I'll send out v2 to fix the issues. -----Original Message----- From: Matt Turner [mailto:[email protected]] Sent: Thursday, July 30, 2015 1:37 AM To: Meng, Mengmeng Cc: [email protected] Subject: Re: [Beignet] [PATCH] fix a powr function issue in cpu compiler math
On Sun, Jul 19, 2015 at 6:33 AM, Meng Mengmeng <[email protected]> wrote: > In OpenCL spec, gentype powr(gentype x, gentype y). In the meantime, > added edge tests for powr. > --- > utests/utest_math_gen.py | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/utests/utest_math_gen.py b/utests/utest_math_gen.py index > 83edcc3..24ddaa4 100755 > --- a/utests/utest_math_gen.py > +++ b/utests/utest_math_gen.py > @@ -467,14 +467,30 @@ static float pown(float x, int y){ > pownUtests = > func('pown','pown',[pown_input_type1,pown_input_type2],pown_output_typ > e,[pown_input_values1,pown_input_values2],'16 * FLT_ULP', > pown_cpu_func) > > ##### gentype powr(gentype x, gentype y) > - powr_input_values1 = [80, -80, 3.14, -3.14, 0.5, 1, -1, > 0.0,6,1500.24,-1500.24] > - powr_input_values2 = [5,6,7,8,10,11,12,13,14,0,12] > + powr_input_values1 = [80,-80,3.14,1, 1.257,+0, -0,+0,-0, +0, -0, +1, > +1, -80, 0,-0,0,-0, > 'INFINITY','INFINITY',+1,+1,0,2.5,'NAN','NAN','NAN' ] > + powr_input_values2 = [5.5,6,7, +0,-0,-1,-15.67,'-INFINITY', '-INFINITY',1, > -2.7,10.5, 3.1415,3.5,-0,-0,0,0, 0, > -0,'INFINITY','-INFINITY','NAN','NAN',-1.5,0,1.5] > powr_input_type1 = ['float','float2','float4','float8','float16'] > powr_input_type2 = ['float','float2','float4','float8','float16'] > powr_output_type = ['float','float2','float4','float8','float16'] > powr_cpu_func=''' > -static float powr(float x, int y){ > - if (x<0) > +static float powr(float x, float y){ > + if (((x > 0) && (x != +INFINITY)) &&((y == -0) || (y == -0))) Space after &&. I think you meant (y == +0) here, but I have more comments below about this. > + return 1; > + else if (((x == +0) || (x == -0)) && ((y <0) || (y == > + -INFINITY))) Space after < > + return +INFINITY; > + else if (((x == +0) || (x == -0)) && (y > 0)) > + return +0; > + else if (((x == +0) || (x == -0)) && ((y == +0) || (y == -0))) > + return NAN; > + else if ((x == +1) && ((y == +INFINITY) || (y == -INFINITY))) > + return NAN; > + else if ((x == +1) && ((y != +INFINITY) && (y != -INFINITY))) > + return 1; > + else if ((x == +INFINITY) && ((y == +0) || (y == -0))) This pattern of (y == +0) || (y == -0) is meaningless for a few reasons: Float == comparison against 0.0f is true if the float is positive or negative 0.0f. There's no need to test for +0.0f and -0.0f separately. Also, the literals you've used ("+0", "-0") are integers which are implicitly promoted to float, and since there isn't a negative-zero integer representation, they both evaluate to (y == 0.0f)... which as I said already handles both positive and negative zero. The code should simply be (y == 0.0f). (The expression y == 0.0 implicitly promotes y to a double since 0.0 without the suffix is double-precision) > + return NAN; > + else if (isnan(x) || (x < 0)) > + return NAN; > + else if ((x >= 0) && (isnan(y))) > return NAN; > else > return powf(x,y); > -- > 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
