[Mesa-stable] [Mesa-dev] [mesa-dev][PATCH] Remove UINT_AS_FLT, INT_AS_FLT, FLOAT_AS_FLT macros.No functional changes, only bug fixed.
Neil Roberts
neil at linux.intel.com
Wed Jan 21 05:45:57 PST 2015
Marius, the ‘Reviewed-by’ tag should only be added if someone explicitly
replies to your patch and says that you can add it with their name. It's
supposed to mean that the person is happy for the patch to be pushed to
master. I did not do this, I only looked at a previous version of the
patch briefly and replied saying that it doesn't appear to fix the
problem. I gave some suggestions of where it might need further work.
Regards,
- Neil
marius.predut at intel.com writes:
> From: Marius Predut <marius.predut at intel.com>
>
> On 32-bit, for floating point operations is used x86 FPU registers instead SSE,
> reason for when reinterprets an integer as a float result is unexpected
> (modify floats when they are written to memory).
> This fixes https://bugs.freedesktop.org/show_bug.cgi?id=82668
>
> Reviewed-by: Roberts, Neil S<neil.s.roberts at intel.com>
> ---
> src/mesa/main/context.c | 2 +-
> src/mesa/main/macros.h | 29 ++-------------------------
> src/mesa/vbo/vbo_attrib_tmp.h | 43 +++++++++++++++++++++++++++++++++++++----
> src/mesa/vbo/vbo_exec.h | 3 ++-
> src/mesa/vbo/vbo_exec_api.c | 25 ++++++++++++------------
> src/mesa/vbo/vbo_exec_eval.c | 22 ++++++++++++++++-----
> src/mesa/vbo/vbo_save_api.c | 10 +++++-----
> 7 files changed, 78 insertions(+), 56 deletions(-)
>
> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> index 400c158..3007491 100644
> --- a/src/mesa/main/context.c
> +++ b/src/mesa/main/context.c
> @@ -656,7 +656,7 @@ _mesa_init_constants(struct gl_constants *consts, gl_api api)
> consts->MaxSamples = 0;
>
> /* GLSL default if NativeIntegers == FALSE */
> - consts->UniformBooleanTrue = FLT_AS_UINT(1.0f);
> + consts->UniformBooleanTrue = 1;
>
> /* GL_ARB_sync */
> consts->MaxServerWaitTimeout = 0x1fff7fffffffULL;
> diff --git a/src/mesa/main/macros.h b/src/mesa/main/macros.h
> index cd5f2d6..4d245e1 100644
> --- a/src/mesa/main/macros.h
> +++ b/src/mesa/main/macros.h
> @@ -170,27 +170,6 @@ extern GLfloat _mesa_ubyte_to_float_color_tab[256];
> ub = ((GLubyte) F_TO_I((f) * 255.0F))
> #endif
>
> -static inline GLfloat INT_AS_FLT(GLint i)
> -{
> - fi_type tmp;
> - tmp.i = i;
> - return tmp.f;
> -}
> -
> -static inline GLfloat UINT_AS_FLT(GLuint u)
> -{
> - fi_type tmp;
> - tmp.u = u;
> - return tmp.f;
> -}
> -
> -static inline unsigned FLT_AS_UINT(float f)
> -{
> - fi_type tmp;
> - tmp.f = f;
> - return tmp.u;
> -}
> -
> /**
> * Convert a floating point value to an unsigned fixed point value.
> *
> @@ -625,15 +604,11 @@ COPY_CLEAN_4V_TYPE_AS_FLOAT(GLfloat dst[4], int sz, const GLfloat src[4],
> {
> switch (type) {
> case GL_FLOAT:
> - ASSIGN_4V(dst, 0, 0, 0, 1);
> + ASSIGN_4V(dst, 0.0f, 0.0f, 0.0f, 1.0f);
> break;
> case GL_INT:
> - ASSIGN_4V(dst, INT_AS_FLT(0), INT_AS_FLT(0),
> - INT_AS_FLT(0), INT_AS_FLT(1));
> - break;
> case GL_UNSIGNED_INT:
> - ASSIGN_4V(dst, UINT_AS_FLT(0), UINT_AS_FLT(0),
> - UINT_AS_FLT(0), UINT_AS_FLT(1));
> + ASSIGN_4V(dst, 0, 0, 0, 1);
> break;
> default:
> ASSIGN_4V(dst, 0.0f, 0.0f, 0.0f, 1.0f); /* silence warnings */
> diff --git a/src/mesa/vbo/vbo_attrib_tmp.h b/src/mesa/vbo/vbo_attrib_tmp.h
> index ec66934..a853cb1 100644
> --- a/src/mesa/vbo/vbo_attrib_tmp.h
> +++ b/src/mesa/vbo/vbo_attrib_tmp.h
> @@ -28,6 +28,41 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
> #include "util/u_format_r11g11b10f.h"
> #include "main/varray.h"
>
> +#include "program/prog_parameter.h"
> +
> +
> +static union gl_constant_value UINT_AS_UNION(GLuint u)
> +{
> + union gl_constant_value tmp;
> + tmp.u = u;
> + return tmp;
> +}
> +
> +static inline union gl_constant_value INT_AS_UNION(GLint i)
> +{
> + union gl_constant_value tmp;
> + tmp.i = i;
> + return tmp;
> +}
> +
> +static inline union gl_constant_value FLOAT_AS_UNION(GLfloat f)
> +{
> + union gl_constant_value tmp;
> + tmp.f = f;
> + return tmp;
> +}
> +
> +/* ATTR */
> +#define ATTR( A, N, T, V0, V1, V2, V3 ) ATTR_##T((A), (N), (T), (V0), (V1), (V2), (V3))
> +
> +#define ATTR_GL_UNSIGNED_INT( A, N, T, V0, V1, V2, V3 ) \
> + ATTR_UNION(A, N, T, UINT_AS_UNION(V0), UINT_AS_UNION(V1), UINT_AS_UNION(V2), UINT_AS_UNION(V3))
> +#define ATTR_GL_INT( A, N, T, V0, V1, V2, V3 ) \
> + ATTR_UNION(A, N, T, INT_AS_UNION(V0), INT_AS_UNION(V1), INT_AS_UNION(V2), INT_AS_UNION(V3))
> +#define ATTR_GL_FLOAT( A, N, T, V0, V1, V2, V3 ) \
> + ATTR_UNION(A, N, T, FLOAT_AS_UNION(V0), FLOAT_AS_UNION(V1), FLOAT_AS_UNION(V2), FLOAT_AS_UNION(V3))
> +
> +
> /* float */
> #define ATTR1FV( A, V ) ATTR( A, 1, GL_FLOAT, (V)[0], 0, 0, 1 )
> #define ATTR2FV( A, V ) ATTR( A, 2, GL_FLOAT, (V)[0], (V)[1], 0, 1 )
> @@ -41,8 +76,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
>
> /* int */
> #define ATTRI( A, N, X, Y, Z, W) ATTR( A, N, GL_INT, \
> - INT_AS_FLT(X), INT_AS_FLT(Y), \
> - INT_AS_FLT(Z), INT_AS_FLT(W) )
> + X, Y, \
> + Z, W )
>
> #define ATTR2IV( A, V ) ATTRI( A, 2, (V)[0], (V)[1], 0, 1 )
> #define ATTR3IV( A, V ) ATTRI( A, 3, (V)[0], (V)[1], (V)[2], 1 )
> @@ -56,8 +91,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
>
> /* uint */
> #define ATTRUI( A, N, X, Y, Z, W) ATTR( A, N, GL_UNSIGNED_INT, \
> - UINT_AS_FLT(X), UINT_AS_FLT(Y), \
> - UINT_AS_FLT(Z), UINT_AS_FLT(W) )
> + X, Y, \
> + Z, W )
>
> #define ATTR2UIV( A, V ) ATTRUI( A, 2, (V)[0], (V)[1], 0, 1 )
> #define ATTR3UIV( A, V ) ATTRUI( A, 3, (V)[0], (V)[1], (V)[2], 1 )
> diff --git a/src/mesa/vbo/vbo_exec.h b/src/mesa/vbo/vbo_exec.h
> index bb265de..7db6b6b 100644
> --- a/src/mesa/vbo/vbo_exec.h
> +++ b/src/mesa/vbo/vbo_exec.h
> @@ -38,6 +38,7 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
> #include "vbo.h"
> #include "vbo_attrib.h"
>
> +#include "program/prog_parameter.h"
>
> /**
> * Max number of primitives (number of glBegin/End pairs) per VBO.
> @@ -104,7 +105,7 @@ struct vbo_exec_context
> GLenum attrtype[VBO_ATTRIB_MAX];
> GLubyte active_sz[VBO_ATTRIB_MAX];
>
> - GLfloat *attrptr[VBO_ATTRIB_MAX];
> + gl_constant_value *attrptr[VBO_ATTRIB_MAX];
> struct gl_client_array arrays[VERT_ATTRIB_MAX];
>
> /* According to program mode, the values above plus current
> diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c
> index 5f8250e..b6c1055 100644
> --- a/src/mesa/vbo/vbo_exec_api.c
> +++ b/src/mesa/vbo/vbo_exec_api.c
> @@ -46,7 +46,6 @@ USE OR OTHER DEALINGS IN THE SOFTWARE.
> #include "vbo_context.h"
> #include "vbo_noop.h"
>
> -
> #ifdef ERROR
> #undef ERROR
> #endif
> @@ -163,7 +162,7 @@ static void vbo_exec_copy_to_current( struct vbo_exec_context *exec )
>
> COPY_CLEAN_4V_TYPE_AS_FLOAT(tmp,
> exec->vtx.attrsz[i],
> - exec->vtx.attrptr[i],
> + (GLfloat*)exec->vtx.attrptr[i],
> exec->vtx.attrtype[i]);
>
> if (exec->vtx.attrtype[i] != vbo->currval[i].Type ||
> @@ -216,10 +215,10 @@ vbo_exec_copy_from_current(struct vbo_exec_context *exec)
> for (i = VBO_ATTRIB_POS + 1; i < VBO_ATTRIB_MAX; i++) {
> const GLfloat *current = (GLfloat *) vbo->currval[i].Ptr;
> switch (exec->vtx.attrsz[i]) {
> - case 4: exec->vtx.attrptr[i][3] = current[3];
> - case 3: exec->vtx.attrptr[i][2] = current[2];
> - case 2: exec->vtx.attrptr[i][1] = current[1];
> - case 1: exec->vtx.attrptr[i][0] = current[0];
> + case 4: exec->vtx.attrptr[i][3].f = current[3];
> + case 3: exec->vtx.attrptr[i][2].f = current[2];
> + case 2: exec->vtx.attrptr[i][1].f = current[1];
> + case 1: exec->vtx.attrptr[i][0].f = current[0];
> break;
> }
> }
> @@ -291,7 +290,7 @@ vbo_exec_wrap_upgrade_vertex(struct vbo_exec_context *exec,
>
> for (i = 0 ; i < VBO_ATTRIB_MAX ; i++) {
> if (exec->vtx.attrsz[i]) {
> - exec->vtx.attrptr[i] = tmp;
> + exec->vtx.attrptr[i] = (gl_constant_value*) tmp;
> tmp += exec->vtx.attrsz[i];
> }
> else
> @@ -305,8 +304,8 @@ vbo_exec_wrap_upgrade_vertex(struct vbo_exec_context *exec,
> }
> else {
> /* Just have to append the new attribute at the end */
> - exec->vtx.attrptr[attr] = exec->vtx.vertex +
> - exec->vtx.vertex_size - newSize;
> + exec->vtx.attrptr[attr] = (gl_constant_value*)(exec->vtx.vertex +
> + exec->vtx.vertex_size - newSize);
> }
>
> /* Replay stored vertices to translate them
> @@ -327,7 +326,7 @@ vbo_exec_wrap_upgrade_vertex(struct vbo_exec_context *exec,
>
> if (sz) {
> GLint old_offset = old_attrptr[j] - exec->vtx.vertex;
> - GLint new_offset = exec->vtx.attrptr[j] - exec->vtx.vertex;
> + GLint new_offset = (GLfloat*)exec->vtx.attrptr[j] - exec->vtx.vertex;
>
> if (j == attr) {
> if (oldSize) {
> @@ -383,7 +382,7 @@ vbo_exec_fixup_vertex(struct gl_context *ctx, GLuint attr, GLuint newSize)
> * zeros. Don't need to flush or wrap.
> */
> for (i = newSize; i <= exec->vtx.attrsz[attr]; i++)
> - exec->vtx.attrptr[attr][i-1] = id[i-1];
> + exec->vtx.attrptr[attr][i-1].f = id[i-1];
> }
>
> exec->vtx.active_sz[attr] = newSize;
> @@ -401,7 +400,7 @@ vbo_exec_fixup_vertex(struct gl_context *ctx, GLuint attr, GLuint newSize)
> * This macro is used to implement all the glVertex, glColor, glTexCoord,
> * glVertexAttrib, etc functions.
> */
> -#define ATTR( A, N, T, V0, V1, V2, V3 ) \
> +#define ATTR_UNION( A, N, T, V0, V1, V2, V3 ) \
> do { \
> struct vbo_exec_context *exec = &vbo_context(ctx)->exec; \
> \
> @@ -412,7 +411,7 @@ do { \
> vbo_exec_fixup_vertex(ctx, A, N); \
> \
> { \
> - GLfloat *dest = exec->vtx.attrptr[A]; \
> + gl_constant_value *dest = exec->vtx.attrptr[A]; \
> if (N>0) dest[0] = V0; \
> if (N>1) dest[1] = V1; \
> if (N>2) dest[2] = V2; \
> diff --git a/src/mesa/vbo/vbo_exec_eval.c b/src/mesa/vbo/vbo_exec_eval.c
> index 82f89b9..670e763 100644
> --- a/src/mesa/vbo/vbo_exec_eval.c
> +++ b/src/mesa/vbo/vbo_exec_eval.c
> @@ -33,6 +33,18 @@
> #include "vbo_exec.h"
>
>
> +/** Copy \p SZ elements into a 4-element vector */
> +#define COPY_UNION_SZ_4V(DST, SZ, SRC) \
> +do { \
> + switch (SZ) { \
> + case 4: (DST)[3].f = (SRC)[3]; \
> + case 3: (DST)[2].f = (SRC)[2]; \
> + case 2: (DST)[1].f = (SRC)[1]; \
> + case 1: (DST)[0].f = (SRC)[0]; \
> + } \
> +} while(0)
> +
> +
> static void clear_active_eval1( struct vbo_exec_context *exec, GLuint attr )
> {
> assert(attr < Elements(exec->eval.map1));
> @@ -138,9 +150,9 @@ void vbo_exec_do_EvalCoord1f(struct vbo_exec_context *exec, GLfloat u)
> exec->eval.map1[attr].sz,
> map->Order);
>
> - COPY_SZ_4V( exec->vtx.attrptr[attr],
> - exec->vtx.attrsz[attr],
> - data );
> + COPY_UNION_SZ_4V( exec->vtx.attrptr[attr],
> + exec->vtx.attrsz[attr],
> + data );
> }
> }
>
> @@ -186,7 +198,7 @@ void vbo_exec_do_EvalCoord2f( struct vbo_exec_context *exec,
> exec->eval.map2[attr].sz,
> map->Uorder, map->Vorder);
>
> - COPY_SZ_4V( exec->vtx.attrptr[attr],
> + COPY_UNION_SZ_4V( exec->vtx.attrptr[attr],
> exec->vtx.attrsz[attr],
> data );
> }
> @@ -225,7 +237,7 @@ void vbo_exec_do_EvalCoord2f( struct vbo_exec_context *exec,
> NORMALIZE_3FV(normal);
> normal[3] = 1.0;
>
> - COPY_SZ_4V( exec->vtx.attrptr[VBO_ATTRIB_NORMAL],
> + COPY_UNION_SZ_4V( exec->vtx.attrptr[VBO_ATTRIB_NORMAL],
> exec->vtx.attrsz[VBO_ATTRIB_NORMAL],
> normal );
>
> diff --git a/src/mesa/vbo/vbo_save_api.c b/src/mesa/vbo/vbo_save_api.c
> index 5055c22..2a43f15 100644
> --- a/src/mesa/vbo/vbo_save_api.c
> +++ b/src/mesa/vbo/vbo_save_api.c
> @@ -769,7 +769,7 @@ _save_reset_vertex(struct gl_context *ctx)
> * 3f version won't otherwise set color[3] to 1.0 -- this is the job
> * of the chooser function when switching between Color4f and Color3f.
> */
> -#define ATTR(A, N, T, V0, V1, V2, V3) \
> +#define ATTR_UNION(A, N, T, V0, V1, V2, V3) \
> do { \
> struct vbo_save_context *save = &vbo_context(ctx)->save; \
> \
> @@ -778,10 +778,10 @@ do { \
> \
> { \
> GLfloat *dest = save->attrptr[A]; \
> - if (N>0) dest[0] = V0; \
> - if (N>1) dest[1] = V1; \
> - if (N>2) dest[2] = V2; \
> - if (N>3) dest[3] = V3; \
> + if (N>0) dest[0] = V0.f; \
> + if (N>1) dest[1] = V1.f; \
> + if (N>2) dest[2] = V2.f; \
> + if (N>3) dest[3] = V3.f; \
> save->attrtype[A] = T; \
> } \
> \
> --
> 1.7.9.5
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-stable
mailing list