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;
> +

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

2015-10-07 Thread Kristian Høgsberg Kristensen 
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,
 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;
+   if (INTEL_DEBUG & DEBUG_SHADER_TIME)
+  st_index = brw_get_shader_time_index(brw, prog, NULL, ST_GS);
+
void *mem_ctx = ralloc_context(NULL);
unsigned program_size;
const unsigned *program =
-  brw_gs_emit(brw, prog, , mem_ctx, _size);
+  brw_gs_emit(brw, prog, , mem_ctx, st_index, _size);
if (program == NULL) {
   ralloc_free(mem_ctx);
   return false;
diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4.cpp
index 4b4a216..ff41c81 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
@@ -1943,14 +1943,11 @@ brw_vs_emit(struct brw_context *brw,
 struct brw_vs_prog_data *prog_data,
 struct

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