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
