[Mesa-dev] [PATCH 7/9] i965: Avoid null access in fs_generator::generate_code()

[Juha-Pekka Heikkila]

On 03.07.2014 16:26, Pohjolainen, Topi wrote:
> On Thu, Jul 03, 2014 at 11:13:17AM +0300, Juha-Pekka Heikkila wrote:
>> Avoid null access while printing debug infos. On the same go
>> change local variable name to avoid confusion because there
>> already is class member with same name.
>>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> index 52e88d4..6e201d1 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
>> @@ -1783,9 +1783,13 @@ fs_generator::generate_code(exec_list *instructions)
>>                dispatch_width, before_size / 16, before_size, after_size,
>>                100.0f * (before_size - after_size) / before_size);
>>  
> 
> I had to check the context a bit, just before there is:
> 
>       if (prog) {
>          ...
>       } else if (fp) {
>          ...
>       } else {
>          fprintf(stderr, "Native code for blorp program (SIMD%d dispatch):\n",
>                  dispatch_width);
>       }
> 
> As I remembered you are now addressing the special case of blorp programs.
> After your change we can't dump them anymore (using env setting
> INTEL_DEBUG=blorp).

We should go to dump_assembly even if prog is NULL? When looking at
dump_assembly I see prog being used only at intel_asm_printer around
line 55 where it goes like:

      if (last_annotation_ir != annotation[i].ir) {
         last_annotation_ir = annotation[i].ir;
         if (last_annotation_ir) {
            fprintf(stderr, "   ");
            if (!prog->Instructions)
               fprint_ir(stderr, annotation[i].ir);
            else {
               const struct prog_instruction *pi =
                  (const struct prog_instruction *)annotation[i].ir;
               fprintf(stderr, "%d: ",
                       (int)(pi - prog->Instructions));
               _mesa_fprint_instruction_opt(stderr,
                                            pi,
                                            0, PROG_PRINT_DEBUG, NULL);
            }
            fprintf(stderr, "\n");
         }

Line 55 is that "if (!prog->Instructions)".

"if (last_annotation_ir != annotation[i].ir)" never matches when prog is
also NULL?

>> -      const struct gl_program *prog = fp ? &fp->Base : NULL;
>> +      const struct gl_program *fp_prog = fp ? &fp->Base : NULL;
>> +
>> +      if (fp_prog) {
>> +         dump_assembly(p->store, annotation.ann_count, annotation.ann, brw,
>> +                       fp_prog);
>> +      }
>>  
>> -      dump_assembly(p->store, annotation.ann_count, annotation.ann, brw, prog);
>>        ralloc_free(annotation.ann);
>>     }
>>  }
>> -- 
>> 1.8.1.2
>>