[Mesa-dev] [PATCH 1/2] mesa: remove redundant modulus operation

[Timothy Arceri]

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 <ian.d.romanick at intel.com>
>>
>>>            }
>>>            values += 4;
>>>         } else {
>>>            /* silence valgrind */
>>>
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev