On 24/05/17 10:25, Timothy Arceri wrote: > On 24/05/17 08:07, Ian Romanick wrote
On 05/23/2017 05:01 AM, Timothy Arceri wrote:
The if check above means we can only get here if size is less than 4.
---
  src/mesa/program/prog_parameter.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/mesa/program/prog_parameter.c b/src/mesa/program/prog_parameter.c
index 44e680c..40bc47d 100644
--- a/src/mesa/program/prog_parameter.c
+++ b/src/mesa/program/prog_parameter.c
@@ -260,23 +260,22 @@ _mesa_add_parameter(struct gl_program_parameter_list *paramList, struct gl_program_parameter *p = paramList->Parameters + oldNum + i;
        p->Name = strdup(name ? name : "");
        p->Type = type;
        p->Size = size;
        p->DataType = datatype;
        if (values) {
           if (size >= 4) {
              COPY_4V(paramList->ParameterValues[oldNum + i], values);
           } else {
              /* copy 1, 2 or 3 values */
-            GLuint remaining = size % 4;
-            assert(remaining < 4);
-            for (j = 0; j < remaining; j++) {
+            assert(size < 4);
+            for (j = 0; j < size; j++) {
paramList->ParameterValues[oldNum + i][j].f = values[j].f;
              }
              /* fill in remaining positions with zeros */
              for (; j < 4; j++) {
                 paramList->ParameterValues[oldNum + i][j].f = 0.0f;
              }

It might be interesting to see if just setting all four values to 0.0
before the copy produces better code.  There are a bunch of different
micro optimizations possible depending on what GCC does.

I've actually been looking at whether we can stop allocating 4 components here by default. I think it should be ok to do, state references always pass a size of 4 as do uniforms from asm. This would allow us to allocate the actual size we need for glsl uniforms.

It looks like i965 already skips these excess components and assigns a pointer to static consts zero instead. Seems the gallium change should be simple enough, i915 looks like it could be a little more tricky.

Actually in i965 we only use these asm uniforms glsl just uses the memory we assigned at link time.



Either way, this patch is

Reviewed-by: Ian Romanick <[email protected]>

           }
           values += 4;
        } else {
           /* silence valgrind */


_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to