i965: Store uniform constant values in a gl_constant_value instead of float

The brw_stage_prog_data struct previously contained an array of float pointers
to the values of parameters. These were then copied into a batch buffer to
upload the values using a regular assignment. However the float values were
also being overloaded to store integer values for integer uniforms. This can
break if x87 floating-point registers are used to do the assignment because
the fst instruction tries to fix up invalid float values. If an integer
constant happened to look like an invalid float value then it would get
altered when it was copied into the batch buffer.

This patch changes the pointers to be gl_constant_value instead so that the
assignment should end up copying without any alteration. This also makes it
more obvious that the values being stored here are overloaded for multiple
types.

There are some static asserts where the values are uploaded to ensure that the
size of gl_constant_value is the same as a float.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81150
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index 1bbcf46975733d6901ceda7ac59ef115d22ab92a..157a605365c62eb6fadee3fdffc9abdd955f218b 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -41,6 +41,7 @@
 #include "main/mtypes.h"
 #include "brw_structs.h"
 #include "intel_aub.h"
+#include "program/prog_parameter.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -309,8 +310,8 @@ struct brw_stage_prog_data {
     * These must be the last fields of the struct (see
     * brw_stage_prog_data_compare()).
     */
-   const float **param;
-   const float **pull_param;
+   const gl_constant_value **param;
+   const gl_constant_value **pull_param;
 };
 
 /* Data about a particular attempt to compile a program.  Note that

diff --git a/src/mesa/drivers/dri/i965/brw_curbe.c b/src/mesa/drivers/dri/i965/brw_curbe.c
index 02eda5f743d5d78481c10f05cbe7af82ee3d91d4..1a828edeb969501b35ea7b580f5c629e90b94987 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_curbe.c
+++ b/src/mesa/drivers/dri/i965/brw_curbe.c
@@ -196,7 +196,7 @@ brw_upload_constant_buffer(struct brw_context *brw)
    /* BRW_NEW_CURBE_OFFSETS */
    const GLuint sz = brw->curbe.total_size;
    const GLuint bufsz = sz * 16 * sizeof(GLfloat);
-   GLfloat *buf;
+   gl_constant_value *buf;
    GLuint i;
    gl_clip_plane *clip_planes;
 
@@ -207,6 +207,8 @@ brw_upload_constant_buffer(struct brw_context *brw)
    buf = intel_upload_space(brw, bufsz, 64,
                             &brw->curbe.curbe_bo, &brw->curbe.curbe_offset);
 
+   STATIC_ASSERT(sizeof(gl_constant_value) == sizeof(float));
+
    /* fragment shader constants */
    if (brw->curbe.wm_size) {
       /* BRW_NEW_CURBE_OFFSETS */
@@ -226,10 +228,10 @@ brw_upload_constant_buffer(struct brw_context *brw)
       /* If any planes are going this way, send them all this way:
        */
       for (i = 0; i < 6; i++) {
-        buf[offset + i * 4 + 0] = fixed_plane[i][0];
-        buf[offset + i * 4 + 1] = fixed_plane[i][1];
-        buf[offset + i * 4 + 2] = fixed_plane[i][2];
-        buf[offset + i * 4 + 3] = fixed_plane[i][3];
+        buf[offset + i * 4 + 0].f = fixed_plane[i][0];
+        buf[offset + i * 4 + 1].f = fixed_plane[i][1];
+        buf[offset + i * 4 + 2].f = fixed_plane[i][2];
+        buf[offset + i * 4 + 3].f = fixed_plane[i][3];
       }
 
       /* Clip planes: _NEW_TRANSFORM plus _NEW_PROJECTION to get to
@@ -238,10 +240,10 @@ brw_upload_constant_buffer(struct brw_context *brw)
       clip_planes = brw_select_clip_planes(ctx);
       for (j = 0; j < MAX_CLIP_PLANES; j++) {
         if (ctx->Transform.ClipPlanesEnabled & (1<<j)) {
-           buf[offset + i * 4 + 0] = clip_planes[j][0];
-           buf[offset + i * 4 + 1] = clip_planes[j][1];
-           buf[offset + i * 4 + 2] = clip_planes[j][2];
-           buf[offset + i * 4 + 3] = clip_planes[j][3];
+           buf[offset + i * 4 + 0].f = clip_planes[j][0];
+           buf[offset + i * 4 + 1].f = clip_planes[j][1];
+           buf[offset + i * 4 + 2].f = clip_planes[j][2];
+           buf[offset + i * 4 + 3].f = clip_planes[j][3];
            i++;
         }
       }
@@ -260,7 +262,7 @@ brw_upload_constant_buffer(struct brw_context *brw)
    if (0) {
       for (i = 0; i < sz*16; i+=4)
         fprintf(stderr, "curbe %d.%d: %f %f %f %f\n", i/8, i&4,
-                 buf[i+0], buf[i+1], buf[i+2], buf[i+3]);
+                 buf[i+0].f, buf[i+1].f, buf[i+2].f, buf[i+3].f);
    }
 
    /* Because this provokes an action (ie copy the constants into the

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 35ada42c508f7a2e23d5d01c60e4a8707e842ccf..43aee66f4101108acc6fabdc0f458425dfbc9493 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -961,7 +961,7 @@ fs_visitor::setup_uniform_values(ir_variable *ir)
          slots *= storage->array_elements;
 
       for (unsigned i = 0; i < slots; i++) {
-         stage_prog_data->param[uniforms++] = &storage->storage[i].f;
+         stage_prog_data->param[uniforms++] = &storage->storage[i];
       }
    }
 
@@ -1000,7 +1000,7 @@ fs_visitor::setup_builtin_uniform_values(ir_variable *ir)
         last_swiz = swiz;
 
          stage_prog_data->param[uniforms++] =
-            &fp->Base.Parameters->ParameterValues[index][swiz].f;
+            &fp->Base.Parameters->ParameterValues[index][swiz];
       }
    }
 }
@@ -1794,7 +1794,7 @@ fs_visitor::move_uniform_array_access_to_pull_constants()
           * add it.
           */
          if (pull_constant_loc[uniform] == -1) {
-            const float **values = &stage_prog_data->param[uniform];
+            const gl_constant_value **values = &stage_prog_data->param[uniform];
 
             assert(param_size[uniform]);
 

diff --git a/src/mesa/drivers/dri/i965/brw_fs_fp.cpp b/src/mesa/drivers/dri/i965/brw_fs_fp.cpp
index 8d07be2ee1732c62558c88813b6d727e3e4b08ea..98df29947ef47a07905c789f7cbb404cc284f3b5 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_fs_fp.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_fp.cpp
@@ -583,7 +583,7 @@ fs_visitor::setup_fp_regs()
            p < prog->Parameters->NumParameters; p++) {
          for (unsigned int i = 0; i < 4; i++) {
             stage_prog_data->param[uniforms++] =
-               &prog->Parameters->ParameterValues[p][i].f;
+               &prog->Parameters->ParameterValues[p][i];
          }
       }
    }

diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
index c16401b649b6b40d6f261363b529d8db68be17ca..2d5f18d448c18abecf91108239bbf4c66284aa0c 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
@@ -1639,7 +1639,7 @@ fs_visitor::rescale_texcoord(ir_texture *ir, fs_reg coordinate,
       /* Try to find existing copies of the texrect scale uniforms. */
       for (unsigned i = 0; i < uniforms; i++) {
          if (stage_prog_data->param[i] ==
-             &prog->Parameters->ParameterValues[index][0].f) {
+             &prog->Parameters->ParameterValues[index][0]) {
             scale_x = fs_reg(UNIFORM, i);
             scale_y = fs_reg(UNIFORM, i + 1);
             break;
@@ -1652,9 +1652,9 @@ fs_visitor::rescale_texcoord(ir_texture *ir, fs_reg coordinate,
          scale_y = fs_reg(UNIFORM, uniforms + 1);
 
          stage_prog_data->param[uniforms++] =
-            &prog->Parameters->ParameterValues[index][0].f;
+            &prog->Parameters->ParameterValues[index][0];
          stage_prog_data->param[uniforms++] =
-            &prog->Parameters->ParameterValues[index][1].f;
+            &prog->Parameters->ParameterValues[index][1];
       }
    }
 

diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
index b572b611fd65df041ab2412b904fe374c2179dcc..5f8f39971b9c00fa3db93c7cde4b7b7a153f679d 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
@@ -667,7 +667,7 @@ vec4_visitor::move_push_constants_to_pull_constants()
       pull_constant_loc[i / 4] = -1;
 
       if (i >= max_uniform_components) {
-        const float **values = &stage_prog_data->param[i];
+        const gl_constant_value **values = &stage_prog_data->param[i];
 
         /* Try to find an existing copy of this uniform in the pull
          * constants if it was part of an array access already.
@@ -1490,10 +1490,10 @@ vec4_visitor::setup_uniforms(int reg)
       this->uniform_vector_size[this->uniforms] = 1;
 
       stage_prog_data->param =
-         reralloc(NULL, stage_prog_data->param, const float *, 4);
+         reralloc(NULL, stage_prog_data->param, const gl_constant_value *, 4);
       for (unsigned int i = 0; i < 4; i++) {
         unsigned int slot = this->uniforms * 4 + i;
-        static float zero = 0.0;
+        static gl_constant_value zero = { .f = 0.0 };
         stage_prog_data->param[slot] = &zero;
       }
 

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs.c b/src/mesa/drivers/dri/i965/brw_vec4_gs.c
index 642829160094807578185031a78162f1f9f5be56..8daa555b5321fd53d0f23288d7fe10c40d3cfa8a 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_vec4_gs.c
+++ b/src/mesa/drivers/dri/i965/brw_vec4_gs.c
@@ -65,9 +65,9 @@ do_gs_prog(struct brw_context *brw,
    param_count += MAX_CLIP_PLANES * 4;
 
    c.prog_data.base.base.param =
-      rzalloc_array(NULL, const float *, param_count);
+      rzalloc_array(NULL, const gl_constant_value *, param_count);
    c.prog_data.base.base.pull_param =
-      rzalloc_array(NULL, const float *, param_count);
+      rzalloc_array(NULL, const gl_constant_value *, param_count);
    /* Setting nr_params here NOT to the size of the param and pull_param
     * arrays, but to the number of uniform components vec4_visitor
     * needs. vec4_visitor::setup_uniforms() will set it back to a proper value.

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index 1b46850261644e619559161d8033a19351a9defb..4863cae3618b167731a9029868e3c49595a13d51 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -691,11 +691,11 @@ vec4_visitor::setup_uniform_values(ir_variable *ir)
 
          int i;
          for (i = 0; i < uniform_vector_size[uniforms]; i++) {
-            stage_prog_data->param[uniforms * 4 + i] = &components->f;
+            stage_prog_data->param[uniforms * 4 + i] = components;
             components++;
          }
          for (; i < 4; i++) {
-            static float zero = 0;
+            static gl_constant_value zero = { .f = 0.0 };
             stage_prog_data->param[uniforms * 4 + i] = &zero;
          }
 
@@ -715,7 +715,8 @@ vec4_visitor::setup_uniform_clipplane_values()
       this->userplane[i] = dst_reg(UNIFORM, this->uniforms);
       this->userplane[i].type = BRW_REGISTER_TYPE_F;
       for (int j = 0; j < 4; ++j) {
-         stage_prog_data->param[this->uniforms * 4 + j] = &clip_planes[i][j];
+         stage_prog_data->param[this->uniforms * 4 + j] =
+            (gl_constant_value *) &clip_planes[i][j];
       }
       ++this->uniforms;
    }
@@ -739,7 +740,8 @@ vec4_visitor::setup_builtin_uniform_values(ir_variable *ir)
        */
       int index = _mesa_add_state_reference(this->prog->Parameters,
                                            (gl_state_index *)slots[i].tokens);
-      float *values = &this->prog->Parameters->ParameterValues[index][0].f;
+      gl_constant_value *values =
+         &this->prog->Parameters->ParameterValues[index][0];
 
       assert(this->uniforms < uniform_array_size);
       this->uniform_vector_size[this->uniforms] = 0;
@@ -3309,7 +3311,8 @@ vec4_visitor::move_uniform_array_access_to_pull_constants()
          * add it.
          */
         if (pull_constant_loc[uniform] == -1) {
-           const float **values = &stage_prog_data->param[uniform * 4];
+           const gl_constant_value **values =
+               &stage_prog_data->param[uniform * 4];
 
            pull_constant_loc[uniform] = stage_prog_data->nr_pull_params / 4;
 

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_vp.cpp b/src/mesa/drivers/dri/i965/brw_vec4_vp.cpp
index f88a3718301036a3cdac5827e3899da4e3d881f3..85f2de577fc5fd77fe51b5cf1333063cc784f5e8 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_vec4_vp.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_vp.cpp
@@ -401,7 +401,7 @@ vec4_vs_visitor::emit_program_code()
       unsigned i;
       for (i = 0; i < params->NumParameters * 4; i++) {
          stage_prog_data->pull_param[i] =
-            &params->ParameterValues[i / 4][i % 4].f;
+            &params->ParameterValues[i / 4][i % 4];
       }
       stage_prog_data->nr_pull_params = i;
    }
@@ -432,7 +432,7 @@ vec4_vs_visitor::setup_vp_regs()
       this->uniform_vector_size[this->uniforms] = components;
       for (unsigned i = 0; i < 4; i++) {
          stage_prog_data->param[this->uniforms * 4 + i] = i >= components
-            ? 0 : &plist->ParameterValues[p][i].f;
+            ? 0 : &plist->ParameterValues[p][i];
       }
       this->uniforms++; /* counted in vec4 units */
    }

diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
index 19b1d3b1a12d3c6aff8b7af52e14efd37a653c11..4574c3ececaf7312bbe07d9cafaf54e1916dcb77 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_vs.c
+++ b/src/mesa/drivers/dri/i965/brw_vs.c
@@ -233,8 +233,10 @@ do_vs_prog(struct brw_context *brw,
     */
    param_count += c.key.base.nr_userclip_plane_consts * 4;
 
-   stage_prog_data->param = rzalloc_array(NULL, const float *, param_count);
-   stage_prog_data->pull_param = rzalloc_array(NULL, const float *, param_count);
+   stage_prog_data->param =
+      rzalloc_array(NULL, const gl_constant_value *, param_count);
+   stage_prog_data->pull_param =
+      rzalloc_array(NULL, const gl_constant_value *, param_count);
 
    /* Setting nr_params here NOT to the size of the param and pull_param
     * arrays, but to the number of uniform components vec4_visitor

diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
index ef4a77a63edbb6adc64f1a73de7c0ee31c297985..1cc96cf683f9213bcd058d69d8a0d690462988f8 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
@@ -76,8 +76,10 @@ brw_upload_pull_constants(struct brw_context *brw,
    uint32_t size = prog_data->nr_pull_params * 4;
    drm_intel_bo *const_bo = NULL;
    uint32_t const_offset;
-   float *constants = intel_upload_space(brw, size, 64,
-                                         &const_bo, &const_offset);
+   gl_constant_value *constants = intel_upload_space(brw, size, 64,
+                                                     &const_bo, &const_offset);
+
+   STATIC_ASSERT(sizeof(gl_constant_value) == sizeof(float));
 
    for (i = 0; i < prog_data->nr_pull_params; i++) {
       constants[i] = *prog_data->pull_param[i];
@@ -85,9 +87,9 @@ brw_upload_pull_constants(struct brw_context *brw,
 
    if (0) {
       for (i = 0; i < ALIGN(prog_data->nr_pull_params, 4) / 4; i++) {
-        float *row = &constants[i * 4];
+        const gl_constant_value *row = &constants[i * 4];
         fprintf(stderr, "const surface %3d: %4.3f %4.3f %4.3f %4.3f\n",
-                 i, row[0], row[1], row[2], row[3]);
+                 i, row[0].f, row[1].f, row[2].f, row[3].f);
       }
    }
 

diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
index 6e068d381b772da980c0b37c64a5a847ed461a01..2e3cd4bb2cc20ab2784bb902b9d797c746038d58 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_wm.c
+++ b/src/mesa/drivers/dri/i965/brw_wm.c
@@ -169,9 +169,10 @@ bool do_wm_prog(struct brw_context *brw,
    }
    /* The backend also sometimes adds params for texture size. */
    param_count += 2 * ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxTextureImageUnits;
-   prog_data.base.param = rzalloc_array(NULL, const float *, param_count);
+   prog_data.base.param =
+      rzalloc_array(NULL, const gl_constant_value *, param_count);
    prog_data.base.pull_param =
-   rzalloc_array(NULL, const float *, param_count);
+      rzalloc_array(NULL, const gl_constant_value *, param_count);
    prog_data.base.nr_params = param_count;
 
    prog_data.barycentric_interp_modes =

diff --git a/src/mesa/drivers/dri/i965/gen6_vs_state.c b/src/mesa/drivers/dri/i965/gen6_vs_state.c
index 905e123837ffb949f1e2640580b0c7604ab33529..77f566cd3566b08abc31b1aa80958da3666c722e 100644 (file)
--- a/src/mesa/drivers/dri/i965/gen6_vs_state.c
+++ b/src/mesa/drivers/dri/i965/gen6_vs_state.c
@@ -67,13 +67,15 @@ gen6_upload_push_constants(struct brw_context *brw,
    if (prog_data->nr_params == 0) {
       stage_state->push_const_size = 0;
    } else {
-      float *param;
+      gl_constant_value *param;
       int i;
 
       param = brw_state_batch(brw, type,
-                             prog_data->nr_params * sizeof(float),
+                             prog_data->nr_params * sizeof(gl_constant_value),
                              32, &stage_state->push_const_offset);
 
+      STATIC_ASSERT(sizeof(gl_constant_value) == sizeof(float));
+
       /* _NEW_PROGRAM_CONSTANTS
        *
        * Also _NEW_TRANSFORM -- we may reference clip planes other than as a
@@ -91,7 +93,7 @@ gen6_upload_push_constants(struct brw_context *brw,
            if ((i & 7) == 0)
               fprintf(stderr, "g%d: ",
                        prog_data->dispatch_grf_start_reg + i / 8);
-           fprintf(stderr, "%8f ", param[i]);
+           fprintf(stderr, "%8f ", param[i].f);
            if ((i & 7) == 7)
               fprintf(stderr, "\n");
         }