Re: [Mesa-dev] [PATCH 09/12] i965: Move brw_get_shader_time_index() call out of emit functions

2015-10-08 Thread Jason Ekstrand 
On Wed, Oct 7, 2015 at 7:11 AM, Kristian Høgsberg Kristensen
 wrote:
> brw_get_shader_time_index() is all tangled up in brw_context state and
> we can't call it from the compiler. Thanks the Jasons recent
> refactoring, we can just get the index and pass to the emit functions
> instead.
>
> Signed-off-by: Kristian Høgsberg Kristensen 
> ---
>  src/mesa/drivers/dri/i965/brw_cs.c|  6 +-
>  src/mesa/drivers/dri/i965/brw_cs.h|  1 +
>  src/mesa/drivers/dri/i965/brw_fs.cpp  | 12 ++--
>  src/mesa/drivers/dri/i965/brw_gs.c|  6 +-
>  src/mesa/drivers/dri/i965/brw_vec4.cpp|  5 +
>  src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp |  5 +
>  src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h   |  1 +
>  src/mesa/drivers/dri/i965/brw_vs.c|  6 +-
>  src/mesa/drivers/dri/i965/brw_vs.h|  1 +
>  src/mesa/drivers/dri/i965/brw_wm.c|  8 +++-
>  src/mesa/drivers/dri/i965/brw_wm.h|  1 +
>  11 files changed, 30 insertions(+), 22 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_cs.c 
> b/src/mesa/drivers/dri/i965/brw_cs.c
> index 34680ee..45fb816 100644
> --- a/src/mesa/drivers/dri/i965/brw_cs.c
> +++ b/src/mesa/drivers/dri/i965/brw_cs.c
> @@ -101,8 +101,12 @@ brw_codegen_cs_prog(struct brw_context *brw,
> if (unlikely(INTEL_DEBUG & DEBUG_CS))
>brw_dump_ir("compute", prog, >base, >program.Base);
>
> +   int st_index = -1;
> +   if (INTEL_DEBUG & DEBUG_SHADER_TIME)
> +  st_index = brw_get_shader_time_index(brw, prog, >program.Base, 
> ST_CS);
> +
> program = brw_cs_emit(brw, mem_ctx, key, _data,
> - >program, prog, _size);
> + >program, prog, st_index, _size);
> if (program == NULL) {
>ralloc_free(mem_ctx);
>return false;
> diff --git a/src/mesa/drivers/dri/i965/brw_cs.h 
> b/src/mesa/drivers/dri/i965/brw_cs.h
> index c07eb6c..80f6e4c 100644
> --- a/src/mesa/drivers/dri/i965/brw_cs.h
> +++ b/src/mesa/drivers/dri/i965/brw_cs.h
> @@ -46,6 +46,7 @@ brw_cs_emit(struct brw_context *brw,
>  struct brw_cs_prog_data *prog_data,
>  struct gl_compute_program *cp,
>  struct gl_shader_program *prog,
> +int st_index,

Can we give the function parameters a more descriptive name such as
shader_time_index?  I don't care about the local temporaries as those
are documented by the fact that you immediately assign the result of
get_shader_time_index() to them.  However, the function arguments are
less obvious.  (Also, FWIW, it'll help me in rebasing because that's
what I named them.)

With that, and matt's whitespace change,

Reviewed-by: Jason Ekstrand 

>  unsigned *final_assembly_size);
>
>  void
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp 
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index b4125aa..65c3628 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -5121,14 +5121,9 @@ brw_wm_fs_emit(struct brw_context *brw,
> struct brw_wm_prog_data *prog_data,
> struct gl_fragment_program *fp,
> struct gl_shader_program *prog,
> +   int st_index8, int st_index16,
> unsigned *final_assembly_size)
>  {
> -   int st_index8 = -1, st_index16 = -1;
> -   if (INTEL_DEBUG & DEBUG_SHADER_TIME) {
> -  st_index8 = brw_get_shader_time_index(brw, prog, >Base, ST_FS8);
> -  st_index16 = brw_get_shader_time_index(brw, prog, >Base, ST_FS16);
> -   }
> -
> /* Now the main event: Visit the shader IR and generate our FS IR for it.
>  */
> fs_visitor v(brw->intelScreen->compiler, brw, mem_ctx, key,
> @@ -5273,6 +5268,7 @@ brw_cs_emit(struct brw_context *brw,
>  struct brw_cs_prog_data *prog_data,
>  struct gl_compute_program *cp,
>  struct gl_shader_program *prog,
> +int st_index,
>  unsigned *final_assembly_size)
>  {
> prog_data->local_size[0] = cp->LocalSize[0];
> @@ -5284,10 +5280,6 @@ brw_cs_emit(struct brw_context *brw,
> cfg_t *cfg = NULL;
> const char *fail_msg = NULL;
>
> -   int st_index = -1;
> -   if (INTEL_DEBUG & DEBUG_SHADER_TIME)
> -  st_index = brw_get_shader_time_index(brw, prog, >Base, ST_CS);
> -
> /* Now the main event: Visit the shader IR and generate our CS IR for it.
>  */
> fs_visitor v8(brw->intelScreen->compiler, brw, mem_ctx, key,
> diff --git a/src/mesa/drivers/dri/i965/brw_gs.c 
> b/src/mesa/drivers/dri/i965/brw_gs.c
> index 26c91e4..e0165fb 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs.c
> @@ -294,10 +294,14 @@ brw_codegen_gs_prog(struct brw_context *brw,
> if (unlikely(INTEL_DEBUG & DEBUG_GS))
>brw_dump_ir("geometry", prog, gs, NULL);
>
> +   int st_index = -1;
> +

Re: [Mesa-dev] [PATCH 09/12] i965: Move brw_get_shader_time_index() call out of emit functions

2015-10-07 Thread Matt Turner 
On Wed, Oct 7, 2015 at 7:11 AM, Kristian Høgsberg Kristensen
 wrote:
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c 
> b/src/mesa/drivers/dri/i965/brw_wm.c
> index 242114f..1d020f6 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -224,8 +224,14 @@ brw_codegen_wm_prog(struct brw_context *brw,
> if (unlikely(INTEL_DEBUG & DEBUG_WM) && fs->base.ir)
>brw_dump_ir("fragment", prog, >base, >program.Base);
>
> +   int st_index8 = -1, st_index16 = -1;
> +   if (INTEL_DEBUG & DEBUG_SHADER_TIME) {
> +  st_index8 = brw_get_shader_time_index(brw, prog, >program.Base, 
> ST_FS8);
> +  st_index16 = brw_get_shader_time_index(brw, prog, >program.Base, 
> ST_FS16);
> +   }
> +
> program = brw_wm_fs_emit(brw, mem_ctx, key, _data,
> ->program, prog, _size);
> +>program, prog, st_index8, 
> st_index16,_size);

Missing space after comma.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev