[Mesa-dev] [PATCH 02/14] i965: Generalize fs_generator further

[Matt Turner]

On Tue, Oct 28, 2014 at 3:17 PM, Kristian Høgsberg <krh at bitplanet.net> wrote:
> This removes all stage specific data from the generator, and lets us
> create a generator for any stage.
>
> Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>
> ---
>  src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp |  5 ++---
>  src/mesa/drivers/dri/i965/brw_fs.cpp            |  2 +-
>  src/mesa/drivers/dri/i965/brw_fs.h              |  7 +++----
>  src/mesa/drivers/dri/i965/brw_fs_generator.cpp  | 20 +++++++-------------
>  4 files changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> index 3afe0e7..7e02781 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit_eu.cpp
> @@ -29,9 +29,8 @@
>  brw_blorp_eu_emitter::brw_blorp_eu_emitter(struct brw_context *brw,
>                                             bool debug_flag)
>     : mem_ctx(ralloc_context(NULL)),
> -     generator(brw, mem_ctx,
> -               rzalloc(mem_ctx, struct brw_wm_prog_key),
> -               rzalloc(mem_ctx, struct brw_wm_prog_data),
> +     generator(brw, mem_ctx, (void *) rzalloc(mem_ctx, struct brw_wm_prog_key),
> +               (struct brw_stage_prog_data *) rzalloc(mem_ctx, struct brw_wm_prog_data),
>                 NULL, NULL, false, debug_flag)
>  {
>  }
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index aa1d8d2..b3684bc 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -3744,7 +3744,7 @@ brw_wm_fs_emit(struct brw_context *brw,
>     }
>
>     const unsigned *assembly = NULL;
> -   fs_generator g(brw, mem_ctx, key, prog_data, prog, fp,
> +   fs_generator g(brw, mem_ctx, (void *) key, &prog_data->base, prog, &fp->Base,
>                    v.runtime_check_aads_emit, INTEL_DEBUG & DEBUG_WM);
>     assembly = g.generate_assembly(simd8_cfg, simd16_cfg,
>                                    final_assembly_size);
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
> index 67956bc..3c3e0d4 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -694,10 +694,10 @@ class fs_generator
>  public:
>     fs_generator(struct brw_context *brw,
>                  void *mem_ctx,
> -                const struct brw_wm_prog_key *key,
> -                struct brw_wm_prog_data *prog_data,
> +                const void *key,
> +                struct brw_stage_prog_data *prog_data,
>                  struct gl_shader_program *shader_prog,
> -                struct gl_fragment_program *fp,
> +                struct gl_program *fp,
>                  bool runtime_check_aads_emit,
>                  bool debug_flag);
>     ~fs_generator();
> @@ -801,7 +801,6 @@ private:
>     struct gl_context *ctx;
>
>     struct brw_compile *p;
> -   gl_shader_stage stage;
>     const void * const key;
>     struct brw_stage_prog_data * const prog_data;
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index c95beb6..06b94dd 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -38,16 +38,16 @@ extern "C" {
>
>  fs_generator::fs_generator(struct brw_context *brw,
>                             void *mem_ctx,
> -                           const struct brw_wm_prog_key *key,
> -                           struct brw_wm_prog_data *prog_data,
> +                           const void *key,
> +                           struct brw_stage_prog_data *prog_data,
>                             struct gl_shader_program *shader_prog,
> -                           struct gl_fragment_program *fp,
> +                           struct gl_program *prog,
>                             bool runtime_check_aads_emit,
>                             bool debug_flag)
>
> -   : brw(brw), stage(MESA_SHADER_FRAGMENT), key(key),
> -     prog_data(&prog_data->base), shader_prog(shader_prog),
> -     prog(&fp->Base), runtime_check_aads_emit(runtime_check_aads_emit),
> +   : brw(brw), key(key),
> +     prog_data(prog_data), shader_prog(shader_prog),
> +     prog(prog), runtime_check_aads_emit(runtime_check_aads_emit),
>       debug_flag(debug_flag), mem_ctx(mem_ctx)
>  {
>     ctx = &brw->ctx;
> @@ -105,7 +105,6 @@ fs_generator::fire_fb_write(fs_inst *inst,
>  {
>     uint32_t msg_control;
>
> -   assert(stage == MESA_SHADER_FRAGMENT);

I like removing these asserts from the function bodies, but I'm
confused why you're doing it. The VS isn't going to call
fire_fb_write, or emit a derivative instruction.

In a separate clean up patch, I'd move these asserts to the
appropriate places in the switch statement (and completely remove the
useless ones from fire_fb_write, which is only called by
generate_fb_write which already has its own copy of the assert).