[Mesa-dev] [PATCH 12/13] mesa/math: Avoid double promotion.

Iago Toral itoral at igalia.com
Tue Jul 14 04:39:02 PDT 2015


On Tue, 2015-07-14 at 13:19 +0200, Iago Toral wrote:
> On Mon, 2015-07-13 at 16:22 -0700, Matt Turner wrote:
> > ---
> >  src/mesa/math/m_clip_tmp.h | 20 ++++++-------
> >  src/mesa/math/m_matrix.c   | 70 +++++++++++++++++++++++-----------------------
> >  src/mesa/math/m_norm_tmp.h |  2 +-
> >  3 files changed, 46 insertions(+), 46 deletions(-)
> > 
> > diff --git a/src/mesa/math/m_clip_tmp.h b/src/mesa/math/m_clip_tmp.h
> > index e289be7..60c0004 100644
> > --- a/src/mesa/math/m_clip_tmp.h
> > +++ b/src/mesa/math/m_clip_tmp.h
> > @@ -194,13 +194,13 @@ static GLvector4f * TAG(cliptest_points3)( GLvector4f *clip_vec,
> >     STRIDE_LOOP {
> >        const GLfloat cx = from[0], cy = from[1], cz = from[2];
> >        GLubyte mask = 0;
> > -      if (cx >  1.0)       mask |= CLIP_RIGHT_BIT;
> > -      else if (cx < -1.0)  mask |= CLIP_LEFT_BIT;
> > -      if (cy >  1.0)       mask |= CLIP_TOP_BIT;
> > -      else if (cy < -1.0)  mask |= CLIP_BOTTOM_BIT;
> > +      if (cx >  1.0F)       mask |= CLIP_RIGHT_BIT;
> > +      else if (cx < -1.0F)  mask |= CLIP_LEFT_BIT;
> > +      if (cy >  1.0F)       mask |= CLIP_TOP_BIT;
> > +      else if (cy < -1.0F)  mask |= CLIP_BOTTOM_BIT;
> >        if (viewport_z_clip) {
> > -	 if (cz >  1.0)       mask |= CLIP_FAR_BIT;
> > -	 else if (cz < -1.0)  mask |= CLIP_NEAR_BIT;
> > +	 if (cz >  1.0F)       mask |= CLIP_FAR_BIT;
> > +	 else if (cz < -1.0F)  mask |= CLIP_NEAR_BIT;
> >        }
> >        clipMask[i] = mask;
> >        tmpOrMask |= mask;
> > @@ -230,10 +230,10 @@ static GLvector4f * TAG(cliptest_points2)( GLvector4f *clip_vec,
> >     STRIDE_LOOP {
> >        const GLfloat cx = from[0], cy = from[1];
> >        GLubyte mask = 0;
> > -      if (cx >  1.0)       mask |= CLIP_RIGHT_BIT;
> > -      else if (cx < -1.0)  mask |= CLIP_LEFT_BIT;
> > -      if (cy >  1.0)       mask |= CLIP_TOP_BIT;
> > -      else if (cy < -1.0)  mask |= CLIP_BOTTOM_BIT;
> > +      if (cx >  1.0F)       mask |= CLIP_RIGHT_BIT;
> > +      else if (cx < -1.0F)  mask |= CLIP_LEFT_BIT;
> > +      if (cy >  1.0F)       mask |= CLIP_TOP_BIT;
> > +      else if (cy < -1.0F)  mask |= CLIP_BOTTOM_BIT;
> >        clipMask[i] = mask;
> >        tmpOrMask |= mask;
> >        tmpAndMask &= mask;
> > diff --git a/src/mesa/math/m_matrix.c b/src/mesa/math/m_matrix.c
> > index 6a42c6c..6522200 100644
> > --- a/src/mesa/math/m_matrix.c
> > +++ b/src/mesa/math/m_matrix.c
> > @@ -380,7 +380,7 @@ static GLboolean invert_matrix_general( GLmatrix *mat )
> >     if (fabsf(r3[0])>fabsf(r2[0])) SWAP_ROWS(r3, r2);
> >     if (fabsf(r2[0])>fabsf(r1[0])) SWAP_ROWS(r2, r1);
> >     if (fabsf(r1[0])>fabsf(r0[0])) SWAP_ROWS(r1, r0);
> > -   if (0.0 == r0[0])  return GL_FALSE;
> > +   if (0.0F == r0[0])  return GL_FALSE;
> >  
> >     /* eliminate first variable     */
> >     m1 = r1[0]/r0[0]; m2 = r2[0]/r0[0]; m3 = r3[0]/r0[0];
> > @@ -388,31 +388,31 @@ static GLboolean invert_matrix_general( GLmatrix *mat )
> >     s = r0[2]; r1[2] -= m1 * s; r2[2] -= m2 * s; r3[2] -= m3 * s;
> >     s = r0[3]; r1[3] -= m1 * s; r2[3] -= m2 * s; r3[3] -= m3 * s;
> >     s = r0[4];
> > -   if (s != 0.0) { r1[4] -= m1 * s; r2[4] -= m2 * s; r3[4] -= m3 * s; }
> > +   if (s != 0.0F) { r1[4] -= m1 * s; r2[4] -= m2 * s; r3[4] -= m3 * s; }
> >     s = r0[5];
> > -   if (s != 0.0) { r1[5] -= m1 * s; r2[5] -= m2 * s; r3[5] -= m3 * s; }
> > +   if (s != 0.0F) { r1[5] -= m1 * s; r2[5] -= m2 * s; r3[5] -= m3 * s; }
> >     s = r0[6];
> > -   if (s != 0.0) { r1[6] -= m1 * s; r2[6] -= m2 * s; r3[6] -= m3 * s; }
> > +   if (s != 0.0F) { r1[6] -= m1 * s; r2[6] -= m2 * s; r3[6] -= m3 * s; }
> >     s = r0[7];
> > -   if (s != 0.0) { r1[7] -= m1 * s; r2[7] -= m2 * s; r3[7] -= m3 * s; }
> > +   if (s != 0.0F) { r1[7] -= m1 * s; r2[7] -= m2 * s; r3[7] -= m3 * s; }
> >  
> >     /* choose pivot - or die */
> >     if (fabsf(r3[1])>fabsf(r2[1])) SWAP_ROWS(r3, r2);
> >     if (fabsf(r2[1])>fabsf(r1[1])) SWAP_ROWS(r2, r1);
> > -   if (0.0 == r1[1])  return GL_FALSE;
> > +   if (0.0F == r1[1])  return GL_FALSE;
> >  
> >     /* eliminate second variable */
> >     m2 = r2[1]/r1[1]; m3 = r3[1]/r1[1];
> >     r2[2] -= m2 * r1[2]; r3[2] -= m3 * r1[2];
> >     r2[3] -= m2 * r1[3]; r3[3] -= m3 * r1[3];
> > -   s = r1[4]; if (0.0 != s) { r2[4] -= m2 * s; r3[4] -= m3 * s; }
> > -   s = r1[5]; if (0.0 != s) { r2[5] -= m2 * s; r3[5] -= m3 * s; }
> > -   s = r1[6]; if (0.0 != s) { r2[6] -= m2 * s; r3[6] -= m3 * s; }
> > -   s = r1[7]; if (0.0 != s) { r2[7] -= m2 * s; r3[7] -= m3 * s; }
> > +   s = r1[4]; if (0.0F != s) { r2[4] -= m2 * s; r3[4] -= m3 * s; }
> > +   s = r1[5]; if (0.0F != s) { r2[5] -= m2 * s; r3[5] -= m3 * s; }
> > +   s = r1[6]; if (0.0F != s) { r2[6] -= m2 * s; r3[6] -= m3 * s; }
> > +   s = r1[7]; if (0.0F != s) { r2[7] -= m2 * s; r3[7] -= m3 * s; }
> >  
> >     /* choose pivot - or die */
> >     if (fabsf(r3[2])>fabsf(r2[2])) SWAP_ROWS(r3, r2);
> > -   if (0.0 == r2[2])  return GL_FALSE;
> > +   if (0.0F == r2[2])  return GL_FALSE;
> >  
> >     /* eliminate third variable */
> >     m3 = r3[2]/r2[2];
> > @@ -421,7 +421,7 @@ static GLboolean invert_matrix_general( GLmatrix *mat )
> >     r3[7] -= m3 * r2[7];
> >  
> >     /* last check */
> > -   if (0.0 == r3[3]) return GL_FALSE;
> > +   if (0.0F == r3[3]) return GL_FALSE;
> >  
> >     s = 1.0F/r3[3];             /* now back substitute row 3 */
> >     r3[4] *= s; r3[5] *= s; r3[6] *= s; r3[7] *= s;
> > @@ -490,26 +490,26 @@ static GLboolean invert_matrix_3d_general( GLmatrix *mat )
> >      */
> >     pos = neg = 0.0;
> >     t =  MAT(in,0,0) * MAT(in,1,1) * MAT(in,2,2);
> > -   if (t >= 0.0) pos += t; else neg += t;
> > +   if (t >= 0.0F) pos += t; else neg += t;
> >  
> >     t =  MAT(in,1,0) * MAT(in,2,1) * MAT(in,0,2);
> > -   if (t >= 0.0) pos += t; else neg += t;
> > +   if (t >= 0.0F) pos += t; else neg += t;
> >  
> >     t =  MAT(in,2,0) * MAT(in,0,1) * MAT(in,1,2);
> > -   if (t >= 0.0) pos += t; else neg += t;
> > +   if (t >= 0.0F) pos += t; else neg += t;
> >  
> >     t = -MAT(in,2,0) * MAT(in,1,1) * MAT(in,0,2);
> > -   if (t >= 0.0) pos += t; else neg += t;
> > +   if (t >= 0.0F) pos += t; else neg += t;
> >  
> >     t = -MAT(in,1,0) * MAT(in,0,1) * MAT(in,2,2);
> > -   if (t >= 0.0) pos += t; else neg += t;
> > +   if (t >= 0.0F) pos += t; else neg += t;
> >  
> >     t = -MAT(in,0,0) * MAT(in,2,1) * MAT(in,1,2);
> > -   if (t >= 0.0) pos += t; else neg += t;
> > +   if (t >= 0.0F) pos += t; else neg += t;
> >  
> >     det = pos + neg;
> >  
> > -   if (fabsf(det) < 1e-25)
> > +   if (fabsf(det) < 1e-25F)
> >        return GL_FALSE;
> >  
> >     det = 1.0F / det;
> > @@ -564,7 +564,7 @@ static GLboolean invert_matrix_3d( GLmatrix *mat )
> >                         MAT(in,0,1) * MAT(in,0,1) +
> >                         MAT(in,0,2) * MAT(in,0,2));
> >  
> > -      if (scale == 0.0)
> > +      if (scale == 0.0F)
> >           return GL_FALSE;
> >  
> >        scale = 1.0F / scale;
> > @@ -799,8 +799,8 @@ _math_matrix_rotate( GLmatrix *mat,
> >     GLfloat m[16];
> >     GLboolean optimized;
> >  
> > -   s = (GLfloat) sin( angle * M_PI / 180.0 );
> > -   c = (GLfloat) cos( angle * M_PI / 180.0 );
> > +   s = sinf( angle * M_PI / 180.0 );
> 
> 180.0F
> 
> > +   c = cosf( angle * M_PI / 180.0 );
> 
> 180.0F

I have just realized that M_PI is double so the angle we pass to
cosf/sinf is going to be a double in the end... I wonder if that is what
we want here since in the end it seems we want a float result... but I
guess that is a different topic.

In any case, as it is, is there any gain with your changes? Before we
had  double->float conversion in the result, but now we have a
double->float conversion in the argument to sinf/cosf, right?

Iago

> I guess we will need wrappers for sinf and cosf.
> 
> Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>
> 
> >  
> >     memcpy(m, Identity, sizeof(GLfloat)*16);
> >     optimized = GL_FALSE;
> > @@ -859,7 +859,7 @@ _math_matrix_rotate( GLmatrix *mat,
> >     if (!optimized) {
> >        const GLfloat mag = sqrtf(x * x + y * y + z * z);
> >  
> > -      if (mag <= 1.0e-4) {
> > +      if (mag <= 1.0e-4F) {
> >           /* no rotation, leave mat as-is */
> >           return;
> >        }
> > @@ -1070,7 +1070,7 @@ _math_matrix_scale( GLmatrix *mat, GLfloat x, GLfloat y, GLfloat z )
> >     m[2] *= x;   m[6] *= y;   m[10] *= z;
> >     m[3] *= x;   m[7] *= y;   m[11] *= z;
> >  
> > -   if (fabsf(x - y) < 1e-8 && fabsf(x - z) < 1e-8)
> > +   if (fabsf(x - y) < 1e-8F && fabsf(x - z) < 1e-8F)
> >        mat->flags |= MAT_FLAG_UNIFORM_SCALE;
> >     else
> >        mat->flags |= MAT_FLAG_GENERAL_SCALE;
> > @@ -1206,7 +1206,7 @@ static void analyse_from_scratch( GLmatrix *mat )
> >     GLuint i;
> >  
> >     for (i = 0 ; i < 16 ; i++) {
> > -      if (m[i] == 0.0) mask |= (1<<i);
> > +      if (m[i] == 0.0F) mask |= (1<<i);
> >     }
> >  
> >     if (m[0] == 1.0F) mask |= (1<<16);
> > @@ -1240,12 +1240,12 @@ static void analyse_from_scratch( GLmatrix *mat )
> >        mat->type = MATRIX_2D;
> >  
> >        /* Check for scale */
> > -      if (SQ(mm-1) > SQ(1e-6) ||
> > -	  SQ(m4m4-1) > SQ(1e-6))
> > +      if (SQ(mm-1) > SQ(1e-6F) ||
> > +	  SQ(m4m4-1) > SQ(1e-6F))
> >  	 mat->flags |= MAT_FLAG_GENERAL_SCALE;
> >  
> >        /* Check for rotation */
> > -      if (SQ(mm4) > SQ(1e-6))
> > +      if (SQ(mm4) > SQ(1e-6F))
> >  	 mat->flags |= MAT_FLAG_GENERAL_3D;
> >        else
> >  	 mat->flags |= MAT_FLAG_ROTATION;
> > @@ -1255,9 +1255,9 @@ static void analyse_from_scratch( GLmatrix *mat )
> >        mat->type = MATRIX_3D_NO_ROT;
> >  
> >        /* Check for scale */
> > -      if (SQ(m[0]-m[5]) < SQ(1e-6) &&
> > -	  SQ(m[0]-m[10]) < SQ(1e-6)) {
> > -	 if (SQ(m[0]-1.0) > SQ(1e-6)) {
> > +      if (SQ(m[0]-m[5]) < SQ(1e-6F) &&
> > +	  SQ(m[0]-m[10]) < SQ(1e-6F)) {
> > +	 if (SQ(m[0]-1.0F) > SQ(1e-6F)) {
> >  	    mat->flags |= MAT_FLAG_UNIFORM_SCALE;
> >           }
> >        }
> > @@ -1275,8 +1275,8 @@ static void analyse_from_scratch( GLmatrix *mat )
> >        mat->type = MATRIX_3D;
> >  
> >        /* Check for scale */
> > -      if (SQ(c1-c2) < SQ(1e-6) && SQ(c1-c3) < SQ(1e-6)) {
> > -	 if (SQ(c1-1.0) > SQ(1e-6))
> > +      if (SQ(c1-c2) < SQ(1e-6F) && SQ(c1-c3) < SQ(1e-6F)) {
> > +	 if (SQ(c1-1.0F) > SQ(1e-6F))
> >  	    mat->flags |= MAT_FLAG_UNIFORM_SCALE;
> >  	 /* else no scale at all */
> >        }
> > @@ -1285,10 +1285,10 @@ static void analyse_from_scratch( GLmatrix *mat )
> >        }
> >  
> >        /* Check for rotation */
> > -      if (SQ(d1) < SQ(1e-6)) {
> > +      if (SQ(d1) < SQ(1e-6F)) {
> >  	 CROSS3( cp, m, m+4 );
> >  	 SUB_3V( cp, cp, (m+8) );
> > -	 if (LEN_SQUARED_3FV(cp) < SQ(1e-6))
> > +	 if (LEN_SQUARED_3FV(cp) < SQ(1e-6F))
> >  	    mat->flags |= MAT_FLAG_ROTATION;
> >  	 else
> >  	    mat->flags |= MAT_FLAG_GENERAL_3D;
> > diff --git a/src/mesa/math/m_norm_tmp.h b/src/mesa/math/m_norm_tmp.h
> > index d3ec1c2..6f1db8d 100644
> > --- a/src/mesa/math/m_norm_tmp.h
> > +++ b/src/mesa/math/m_norm_tmp.h
> > @@ -80,7 +80,7 @@ TAG(transform_normalize_normals)( const GLmatrix *mat,
> >        }
> >     }
> >     else {
> > -      if (scale != 1.0) {
> > +      if (scale != 1.0f) {
> >  	 m0 *= scale,  m4 *= scale,  m8 *= scale;
> >  	 m1 *= scale,  m5 *= scale,  m9 *= scale;
> >  	 m2 *= scale,  m6 *= scale,  m10 *= scale;
> 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev




More information about the mesa-dev mailing list