Re: [Mesa-dev] [PATCH 7/9] i965: Avoid null access in fs_generator::generate_code()
On Thu, Jul 03, 2014 at 05:31:19PM +0300, Juha-Pekka Heikkila wrote: 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.heikk...@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? In blorp there are no annotations and hence this path is not taken. So yes, we should allow dumping without prog but then probably just ignore annotations altogether. I think the long term plan is still to get rid of blorp programs and something simple should suffice, I think. - 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 7/9] i965: Avoid null access in fs_generator::generate_code()
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.heikk...@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); - 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 7/9] i965: Avoid null access in fs_generator::generate_code()
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.heikk...@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). - 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 7/9] i965: Avoid null access in fs_generator::generate_code()
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.heikk...@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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev