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

Jason Ekstrand jason at jlekstrand.net
Thu Oct 8 11:49:10 PDT 2015


On Wed, Oct 7, 2015 at 7:11 AM, Kristian Høgsberg Kristensen
<krh at bitplanet.net> 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 <krh at bitplanet.net>
> ---
>  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, &cs->base, &cp->program.Base);
>
> +   int st_index = -1;
> +   if (INTEL_DEBUG & DEBUG_SHADER_TIME)
> +      st_index = brw_get_shader_time_index(brw, prog, &cp->program.Base, ST_CS);
> +
>     program = brw_cs_emit(brw, mem_ctx, key, &prog_data,
> -                         &cp->program, prog, &program_size);
> +                         &cp->program, prog, st_index, &program_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 <jason.ekstrand at intel.com>

>              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, &fp->Base, ST_FS8);
> -      st_index16 = brw_get_shader_time_index(brw, prog, &fp->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, &cp->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, &c, mem_ctx, &program_size);
> +      brw_gs_emit(brw, prog, &c, mem_ctx, st_index, &program_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 gl_vertex_program *vp,
>              struct gl_shader_program *prog,
> +            int st_index,
>              unsigned *final_assembly_size)
>  {
>     const unsigned *assembly = NULL;
>
> -   int st_index = -1;
> -   if (INTEL_DEBUG & DEBUG_SHADER_TIME)
> -      st_index = brw_get_shader_time_index(brw, prog, &vp->Base, ST_VS);
> -
>     if (brw->intelScreen->compiler->scalar_vs) {
>        prog_data->base.dispatch_mode = DISPATCH_MODE_SIMD8;
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> index 36ab25d..4d27fb4 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp
> @@ -618,14 +618,11 @@ brw_gs_emit(struct brw_context *brw,
>              struct gl_shader_program *prog,
>              struct brw_gs_compile *c,
>              void *mem_ctx,
> +            int st_index,
>              unsigned *final_assembly_size)
>  {
>     struct gl_shader *shader = prog->_LinkedShaders[MESA_SHADER_GEOMETRY];
>
> -   int st_index = -1;
> -   if (INTEL_DEBUG & DEBUG_SHADER_TIME)
> -      st_index = brw_get_shader_time_index(brw, prog, NULL, ST_GS);
> -
>     if (brw->gen >= 7) {
>        /* Compile the geometry shader in DUAL_OBJECT dispatch mode, if we can do
>         * so without spilling. If the GS invocations count > 1, then we can't use
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h
> index da93f0d..529e768 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.h
> @@ -55,6 +55,7 @@ const unsigned *brw_gs_emit(struct brw_context *brw,
>                              struct gl_shader_program *prog,
>                              struct brw_gs_compile *c,
>                              void *mem_ctx,
> +                            int st_index,
>                              unsigned *final_assembly_size);
>
>  #ifdef __cplusplus
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
> index b21b29e..7bf8a07 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -173,10 +173,14 @@ brw_codegen_vs_prog(struct brw_context *brw,
>     if (unlikely(INTEL_DEBUG & DEBUG_VS) && vs->base.ir)
>        brw_dump_ir("vertex", prog, &vs->base, &vp->program.Base);
>
> +   int st_index = -1;
> +   if (INTEL_DEBUG & DEBUG_SHADER_TIME)
> +      st_index = brw_get_shader_time_index(brw, prog, &vp->program.Base, ST_VS);
> +
>     /* Emit GEN4 code.
>      */
>     program = brw_vs_emit(brw, mem_ctx, key, &prog_data,
> -                         &vp->program, prog, &program_size);
> +                         &vp->program, prog, st_index, &program_size);
>     if (program == NULL) {
>        ralloc_free(mem_ctx);
>        return false;
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.h b/src/mesa/drivers/dri/i965/brw_vs.h
> index 19551c9..267d9e1 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.h
> +++ b/src/mesa/drivers/dri/i965/brw_vs.h
> @@ -60,6 +60,7 @@ const unsigned *brw_vs_emit(struct brw_context *brw,
>                              struct brw_vs_prog_data *prog_data,
>                              struct gl_vertex_program *vp,
>                              struct gl_shader_program *shader_prog,
> +                            int st_index,
>                              unsigned *program_size);
>  void brw_vs_debug_recompile(struct brw_context *brw,
>                              struct gl_shader_program *prog,
> 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, &fs->base, &fp->program.Base);
>
> +   int st_index8 = -1, st_index16 = -1;
> +   if (INTEL_DEBUG & DEBUG_SHADER_TIME) {
> +      st_index8 = brw_get_shader_time_index(brw, prog, &fp->program.Base, ST_FS8);
> +      st_index16 = brw_get_shader_time_index(brw, prog, &fp->program.Base, ST_FS16);
> +   }
> +
>     program = brw_wm_fs_emit(brw, mem_ctx, key, &prog_data,
> -                            &fp->program, prog, &program_size);
> +                            &fp->program, prog, st_index8, st_index16,&program_size);
>     if (program == NULL) {
>        ralloc_free(mem_ctx);
>        return false;
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.h b/src/mesa/drivers/dri/i965/brw_wm.h
> index 77b83b0..b9c2607 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.h
> +++ b/src/mesa/drivers/dri/i965/brw_wm.h
> @@ -72,6 +72,7 @@ const unsigned *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);
>
>  GLboolean brw_link_shader(struct gl_context *ctx, struct gl_shader_program *prog);
> --
> 2.4.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list