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

2014-07-04 Thread Pohjolainen, Topi 
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()

2014-07-03 Thread Juha-Pekka Heikkila 
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()

2014-07-03 Thread Pohjolainen, Topi 
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()

2014-07-03 Thread 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.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