Re: [Mesa-dev] [PATCH 1/2] i965: Stop doing our optimization on a copy of the GLSL IR.

2014-01-17 Thread Ian Romanick
On 01/13/2014 10:28 PM, Eric Anholt wrote:
 The original intent was that we'd keep a driver-private copy, and there
 would be the normal copy for swrast to make use of without the tuning (or
 anything more invasive we might do) specific to i965.  Only, we don't
 generate swrast code any more, because swrast can't render current shaders
 anyway.  Thus, our private copy is rather a waste, and we can just do our
 backend-specific operations on the linked shader.

I was thinking there were some other goofy issues (interactions of
glLinkProgram w/o glUseProgram, and releasing the IR after
glDeleteProgram), but neither of those appear to be problems.

Both patches are

Reviewed-by: Ian Romanick ian.d.roman...@intel.com

 ---
  src/mesa/drivers/dri/i965/brw_context.h   |  3 --
  src/mesa/drivers/dri/i965/brw_fs.cpp  |  4 +-
  src/mesa/drivers/dri/i965/brw_shader.cpp  | 55 
 ++-
  src/mesa/drivers/dri/i965/brw_vec4.cpp|  4 +-
  src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp |  2 +-
  5 files changed, 28 insertions(+), 40 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
 b/src/mesa/drivers/dri/i965/brw_context.h
 index 63dd4a0..099f2f6 100644
 --- a/src/mesa/drivers/dri/i965/brw_context.h
 +++ b/src/mesa/drivers/dri/i965/brw_context.h
 @@ -320,9 +320,6 @@ struct brw_shader {
 struct gl_shader base;
  
 bool compiled_once;
 -
 -   /** Shader IR transformed for native compile, at link time. */
 -   struct exec_list *ir;
  };
  
  /* Note: If adding fields that need anything besides a normal memcmp() for
 diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
 b/src/mesa/drivers/dri/i965/brw_fs.cpp
 index baf9220..3536cbe 100644
 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
 @@ -3156,7 +3156,7 @@ fs_visitor::run()
 * functions called main).
 */
if (shader) {
 - foreach_list(node, *shader-ir) {
 + foreach_list(node, *shader-base.ir) {
  ir_instruction *ir = (ir_instruction *)node;
  base_ir = ir;
  this-result = reg_undef;
 @@ -3305,7 +3305,7 @@ brw_wm_fs_emit(struct brw_context *brw, struct 
 brw_wm_compile *c,
 if (unlikely(INTEL_DEBUG  DEBUG_WM)) {
if (prog) {
   printf(GLSL IR for native fragment shader %d:\n, prog-Name);
 - _mesa_print_ir(shader-ir, NULL);
 + _mesa_print_ir(shader-base.ir, NULL);
   printf(\n\n);
} else {
   printf(ARB_fragment_program %d ir for native fragment shader\n,
 diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp 
 b/src/mesa/drivers/dri/i965/brw_shader.cpp
 index cf9ca4b..5752348 100644
 --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
 @@ -135,24 +135,18 @@ brw_link_shader(struct gl_context *ctx, struct 
 gl_shader_program *shProg)
  
_mesa_copy_linked_program_data((gl_shader_stage) stage, shProg, prog);
  
 -  void *mem_ctx = ralloc_context(NULL);
bool progress;
  
 -  if (shader-ir)
 -  ralloc_free(shader-ir);
 -  shader-ir = new(shader) exec_list;
 -  clone_ir_list(mem_ctx, shader-ir, shader-base.ir);
 -
/* lower_packing_builtins() inserts arithmetic instructions, so it
 * must precede lower_instructions().
 */
 -  brw_lower_packing_builtins(brw, (gl_shader_stage) stage, shader-ir);
 -  do_mat_op_to_vec(shader-ir);
 +  brw_lower_packing_builtins(brw, (gl_shader_stage) stage, 
 shader-base.ir);
 +  do_mat_op_to_vec(shader-base.ir);
const int bitfield_insert = brw-gen = 7
? BITFIELD_INSERT_TO_BFM_BFI
: 0;
const int lrp_to_arith = brw-gen  6 ? LRP_TO_ARITH : 0;
 -  lower_instructions(shader-ir,
 +  lower_instructions(shader-base.ir,
MOD_TO_FRACT |
DIV_TO_MUL_RCP |
SUB_TO_ADD_NEG |
 @@ -166,17 +160,17 @@ brw_link_shader(struct gl_context *ctx, struct 
 gl_shader_program *shProg)
 * if-statements need to be flattened.
 */
if (brw-gen  6)
 -  lower_if_to_cond_assign(shader-ir, 16);
 -
 -  do_lower_texture_projection(shader-ir);
 -  brw_lower_texture_gradients(brw, shader-ir);
 -  do_vec_index_to_cond_assign(shader-ir);
 -  lower_vector_insert(shader-ir, true);
 -  brw_do_cubemap_normalize(shader-ir);
 -  brw_do_lower_offset_arrays(shader-ir);
 -  brw_do_lower_unnormalized_offset(shader-ir);
 -  lower_noise(shader-ir);
 -  lower_quadop_vector(shader-ir, false);
 +  lower_if_to_cond_assign(shader-base.ir, 16);
 +
 +  do_lower_texture_projection(shader-base.ir);
 +  brw_lower_texture_gradients(brw, shader-base.ir);
 +  do_vec_index_to_cond_assign(shader-base.ir);
 +  lower_vector_insert(shader-base.ir, true);
 +  brw_do_cubemap_normalize(shader-base.ir);
 + 

[Mesa-dev] [PATCH 1/2] i965: Stop doing our optimization on a copy of the GLSL IR.

2014-01-13 Thread Eric Anholt
The original intent was that we'd keep a driver-private copy, and there
would be the normal copy for swrast to make use of without the tuning (or
anything more invasive we might do) specific to i965.  Only, we don't
generate swrast code any more, because swrast can't render current shaders
anyway.  Thus, our private copy is rather a waste, and we can just do our
backend-specific operations on the linked shader.
---
 src/mesa/drivers/dri/i965/brw_context.h   |  3 --
 src/mesa/drivers/dri/i965/brw_fs.cpp  |  4 +-
 src/mesa/drivers/dri/i965/brw_shader.cpp  | 55 ++-
 src/mesa/drivers/dri/i965/brw_vec4.cpp|  4 +-
 src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp |  2 +-
 5 files changed, 28 insertions(+), 40 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.h 
b/src/mesa/drivers/dri/i965/brw_context.h
index 63dd4a0..099f2f6 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -320,9 +320,6 @@ struct brw_shader {
struct gl_shader base;
 
bool compiled_once;
-
-   /** Shader IR transformed for native compile, at link time. */
-   struct exec_list *ir;
 };
 
 /* Note: If adding fields that need anything besides a normal memcmp() for
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
b/src/mesa/drivers/dri/i965/brw_fs.cpp
index baf9220..3536cbe 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -3156,7 +3156,7 @@ fs_visitor::run()
* functions called main).
*/
   if (shader) {
- foreach_list(node, *shader-ir) {
+ foreach_list(node, *shader-base.ir) {
 ir_instruction *ir = (ir_instruction *)node;
 base_ir = ir;
 this-result = reg_undef;
@@ -3305,7 +3305,7 @@ brw_wm_fs_emit(struct brw_context *brw, struct 
brw_wm_compile *c,
if (unlikely(INTEL_DEBUG  DEBUG_WM)) {
   if (prog) {
  printf(GLSL IR for native fragment shader %d:\n, prog-Name);
- _mesa_print_ir(shader-ir, NULL);
+ _mesa_print_ir(shader-base.ir, NULL);
  printf(\n\n);
   } else {
  printf(ARB_fragment_program %d ir for native fragment shader\n,
diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp 
b/src/mesa/drivers/dri/i965/brw_shader.cpp
index cf9ca4b..5752348 100644
--- a/src/mesa/drivers/dri/i965/brw_shader.cpp
+++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
@@ -135,24 +135,18 @@ brw_link_shader(struct gl_context *ctx, struct 
gl_shader_program *shProg)
 
   _mesa_copy_linked_program_data((gl_shader_stage) stage, shProg, prog);
 
-  void *mem_ctx = ralloc_context(NULL);
   bool progress;
 
-  if (shader-ir)
-ralloc_free(shader-ir);
-  shader-ir = new(shader) exec_list;
-  clone_ir_list(mem_ctx, shader-ir, shader-base.ir);
-
   /* lower_packing_builtins() inserts arithmetic instructions, so it
* must precede lower_instructions().
*/
-  brw_lower_packing_builtins(brw, (gl_shader_stage) stage, shader-ir);
-  do_mat_op_to_vec(shader-ir);
+  brw_lower_packing_builtins(brw, (gl_shader_stage) stage, 
shader-base.ir);
+  do_mat_op_to_vec(shader-base.ir);
   const int bitfield_insert = brw-gen = 7
   ? BITFIELD_INSERT_TO_BFM_BFI
   : 0;
   const int lrp_to_arith = brw-gen  6 ? LRP_TO_ARITH : 0;
-  lower_instructions(shader-ir,
+  lower_instructions(shader-base.ir,
 MOD_TO_FRACT |
 DIV_TO_MUL_RCP |
 SUB_TO_ADD_NEG |
@@ -166,17 +160,17 @@ brw_link_shader(struct gl_context *ctx, struct 
gl_shader_program *shProg)
* if-statements need to be flattened.
*/
   if (brw-gen  6)
-lower_if_to_cond_assign(shader-ir, 16);
-
-  do_lower_texture_projection(shader-ir);
-  brw_lower_texture_gradients(brw, shader-ir);
-  do_vec_index_to_cond_assign(shader-ir);
-  lower_vector_insert(shader-ir, true);
-  brw_do_cubemap_normalize(shader-ir);
-  brw_do_lower_offset_arrays(shader-ir);
-  brw_do_lower_unnormalized_offset(shader-ir);
-  lower_noise(shader-ir);
-  lower_quadop_vector(shader-ir, false);
+lower_if_to_cond_assign(shader-base.ir, 16);
+
+  do_lower_texture_projection(shader-base.ir);
+  brw_lower_texture_gradients(brw, shader-base.ir);
+  do_vec_index_to_cond_assign(shader-base.ir);
+  lower_vector_insert(shader-base.ir, true);
+  brw_do_cubemap_normalize(shader-base.ir);
+  brw_do_lower_offset_arrays(shader-base.ir);
+  brw_do_lower_unnormalized_offset(shader-base.ir);
+  lower_noise(shader-base.ir);
+  lower_quadop_vector(shader-base.ir, false);
 
   bool input = true;
   bool output = stage == MESA_SHADER_FRAGMENT;
@@ -184,7 +178,7 @@ brw_link_shader(struct gl_context *ctx, struct 
gl_shader_program *shProg)
   bool uniform = false;