[Mesa-dev] [PATCH 7/7] [RFC] i965/fs: fill allocated memory with zeros where needed

[Juha-Pekka Heikkila]

On 07.06.2016 18:07, Ilia Mirkin wrote:
> On Tue, Jun 7, 2016 at 10:26 AM, Juha-Pekka Heikkila
> <juhapekka.heikkila at gmail.com> wrote:
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
>> ---
>>   src/mesa/drivers/dri/i965/brw_fs.cpp                     | 2 +-
>>   src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp    | 2 +-
>>   src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp | 4 ++--
>>   3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> index 4b29ee5..a04d464 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
>> @@ -2225,7 +2225,7 @@ fs_visitor::assign_constant_locations()
>>      /* As the uniforms are going to be reordered, take the data from a temporary
>>       * copy of the original param[].
>>       */
>> -   gl_constant_value **param = ralloc_array(NULL, gl_constant_value*,
>> +   gl_constant_value **param = rzalloc_array(NULL, gl_constant_value*,
>>                                               stage_prog_data->nr_params);
>>      memcpy(param, stage_prog_data->param,
>>             sizeof(gl_constant_value*) * stage_prog_data->nr_params);
>
> This immediately overwrites all of param with other data. Are you sure
> it's necessary to rzalloc here?
>

You're correct. I did check it is not needed. Without checking this 
could be coming from rebase, oldest pieces on this set is one year old.

>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> index 438f681..11f8576 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
>> @@ -822,7 +822,7 @@ fs_visitor::opt_copy_propagate_local(void *copy_prop_ctx, bblock_t *block,
>>               int regs_written = effective_width *
>>                  type_sz(inst->src[i].type) / REG_SIZE;
>>               if (inst->src[i].file == VGRF) {
>> -               acp_entry *entry = ralloc(copy_prop_ctx, acp_entry);
>> +               acp_entry *entry = rzalloc(copy_prop_ctx, acp_entry);
>
> Why is this necessary here but not above? For the entry->saturate? I
> think it might make sense to just initialize that directly instead of
> doing the zeroing.

This is something where I don't know the answer for. I made this set by 
switching ralloc to use malloc and go crazy with valgrind reports. I did 
think it was enough for now to fix places that break, I mean places that 
seemed to need memory to be zeroed to give them the zeroed memory 
without worrying why is it so.

I ran piglit and glbenchmark on my IVB. For piglit I went after the 
changed tests with valgrind and glbenchmark was just with valgrind all 
over.

>
>>                  entry->dst = inst->dst;
>>                  entry->dst.reg_offset += offset;
>>                  entry->src = inst->src[i];
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
>> index 45f5c5e..9fa259e 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_dead_code_eliminate.cpp
>> @@ -42,8 +42,8 @@ fs_visitor::dead_code_eliminate()
>>      calculate_live_intervals();
>>
>>      int num_vars = live_intervals->num_vars;
>> -   BITSET_WORD *live = ralloc_array(NULL, BITSET_WORD, BITSET_WORDS(num_vars));
>> -   BITSET_WORD *flag_live = ralloc_array(NULL, BITSET_WORD, 1);
>> +   BITSET_WORD *live = rzalloc_array(NULL, BITSET_WORD, BITSET_WORDS(num_vars));
>> +   BITSET_WORD *flag_live = rzalloc_array(NULL, BITSET_WORD, 1);
>>
>>      foreach_block_reverse_safe(block, cfg) {
>>         memcpy(live, live_intervals->block_data[block->num].liveout,
>> --
>> 1.9.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev