On Wed, Jul 01, 2015 at 03:03:36PM -0700, Kenneth Graunke wrote:
> After tearing it out another level or two, and just passing the key and
> vp directly, we can finally remove this struct. It also eliminates a
> pointless memcpy() of the key.
>
> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> ---
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 37
> +++++++++++++++++-----------------
> src/mesa/drivers/dri/i965/brw_vs.c | 20 ++++++++----------
> src/mesa/drivers/dri/i965/brw_vs.h | 13 ++++--------
> 3 files changed, 31 insertions(+), 39 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 42d014c..39715c4 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1872,10 +1872,11 @@ extern "C" {
> */
> const unsigned *
> brw_vs_emit(struct brw_context *brw,
> - struct gl_shader_program *prog,
> - struct brw_vs_compile *c,
> - struct brw_vs_prog_data *prog_data,
> void *mem_ctx,
> + const struct brw_vs_prog_key *key,
> + struct brw_vs_prog_data *prog_data,
> + struct gl_vertex_program *vp,
> + struct gl_shader_program *prog,
> unsigned *final_assembly_size)
> {
> bool start_busy = false;
> @@ -1894,29 +1895,29 @@ brw_vs_emit(struct brw_context *brw,
>
> int st_index = -1;
> if (INTEL_DEBUG & DEBUG_SHADER_TIME)
> - st_index = brw_get_shader_time_index(brw, prog, &c->vp->program.Base,
> + st_index = brw_get_shader_time_index(brw, prog, &vp->Base,
> ST_VS);
This would now fit into the previous line. Two similar insignificant
formatting nits further down, either way:
Reviewed-by: Topi Pohjolainen <topi.pohjolai...@intel.com>
>
> if (unlikely(INTEL_DEBUG & DEBUG_VS))
> - brw_dump_ir("vertex", prog, &shader->base, &c->vp->program.Base);
> + brw_dump_ir("vertex", prog, &shader->base, &vp->Base);
>
> if (brw->intelScreen->compiler->scalar_vs) {
> - if (!c->vp->program.Base.nir) {
> + if (!vp->Base.nir) {
> /* Normally we generate NIR in LinkShader() or
> * ProgramStringNotify(), but Mesa's fixed-function vertex program
> * handling doesn't notify the driver at all. Just do it here, at
> * the last minute, even though it's lame.
> */
> - assert(c->vp->program.Base.Id == 0 && prog == NULL);
> - c->vp->program.Base.nir =
> - brw_create_nir(brw, NULL, &c->vp->program.Base,
> MESA_SHADER_VERTEX);
> + assert(vp->Base.Id == 0 && prog == NULL);
> + vp->Base.nir =
> + brw_create_nir(brw, NULL, &vp->Base, MESA_SHADER_VERTEX);
> }
>
> prog_data->base.dispatch_mode = DISPATCH_MODE_SIMD8;
>
> fs_visitor v(brw->intelScreen->compiler, brw,
> - mem_ctx, MESA_SHADER_VERTEX, &c->key,
> - &prog_data->base.base, prog, &c->vp->program.Base,
> + mem_ctx, MESA_SHADER_VERTEX, key,
> + &prog_data->base.base, prog, &vp->Base,
> 8, st_index);
> if (!v.run_vs(brw_select_clip_planes(&brw->ctx))) {
> if (prog) {
> @@ -1931,8 +1932,8 @@ brw_vs_emit(struct brw_context *brw,
> }
>
> fs_generator g(brw->intelScreen->compiler, brw,
> - mem_ctx, (void *) &c->key, &prog_data->base.base,
> - &c->vp->program.Base, v.promoted_constants,
> + mem_ctx, (void *) key, &prog_data->base.base,
You could drop the extra space before 'key'.
> + &vp->Base, v.promoted_constants,
> v.runtime_check_aads_emit, "VS");
> if (INTEL_DEBUG & DEBUG_VS) {
> char *name;
> @@ -1942,7 +1943,7 @@ brw_vs_emit(struct brw_context *brw,
> prog->Name);
> } else {
> name = ralloc_asprintf(mem_ctx, "vertex program %d",
> - c->vp->program.Base.Id);
> + vp->Base.Id);
> }
> g.enable_debug(name);
> }
> @@ -1953,8 +1954,8 @@ brw_vs_emit(struct brw_context *brw,
> if (!assembly) {
> prog_data->base.dispatch_mode = DISPATCH_MODE_4X2_DUAL_OBJECT;
>
> - vec4_vs_visitor v(brw->intelScreen->compiler, brw, &c->key, prog_data,
> - &c->vp->program, prog, mem_ctx, st_index,
> + vec4_vs_visitor v(brw->intelScreen->compiler, brw, key, prog_data,
> + vp, prog, mem_ctx, st_index,
> !_mesa_is_gles3(&brw->ctx));
> if (!v.run(brw_select_clip_planes(&brw->ctx))) {
> if (prog) {
> @@ -1969,14 +1970,14 @@ brw_vs_emit(struct brw_context *brw,
> }
>
> vec4_generator g(brw->intelScreen->compiler, brw,
> - prog, &c->vp->program.Base, &prog_data->base,
> + prog, &vp->Base, &prog_data->base,
> mem_ctx, INTEL_DEBUG & DEBUG_VS, "vertex", "VS");
> assembly = g.generate_assembly(v.cfg, final_assembly_size);
> }
>
> if (unlikely(brw->perf_debug) && shader) {
> if (shader->compiled_once) {
> - brw_vs_debug_recompile(brw, prog, &c->key);
> + brw_vs_debug_recompile(brw, prog, key);
> }
> if (start_busy && !drm_intel_bo_busy(brw->batch.last_bo)) {
> perf_debug("VS compile took %.03f ms and stalled the GPU\n",
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c
> b/src/mesa/drivers/dri/i965/brw_vs.c
> index ee3f664..2b9b005 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> @@ -94,7 +94,6 @@ brw_codegen_vs_prog(struct brw_context *brw,
> {
> GLuint program_size;
> const GLuint *program;
> - struct brw_vs_compile c;
> struct brw_vs_prog_data prog_data;
> struct brw_stage_prog_data *stage_prog_data = &prog_data.base.base;
> void *mem_ctx;
> @@ -104,8 +103,6 @@ brw_codegen_vs_prog(struct brw_context *brw,
> if (prog)
> vs = prog->_LinkedShaders[MESA_SHADER_VERTEX];
>
> - memset(&c, 0, sizeof(c));
> - memcpy(&c.key, key, sizeof(*key));
> memset(&prog_data, 0, sizeof(prog_data));
>
> /* Use ALT floating point mode for ARB programs so that 0^0 == 1. */
> @@ -114,8 +111,6 @@ brw_codegen_vs_prog(struct brw_context *brw,
>
> mem_ctx = ralloc_context(NULL);
>
> - c.vp = vp;
> -
> /* Allocate the references to the uniforms that will end up in the
> * prog_data associated with the compiled program, and which will be freed
> * by the state cache.
> @@ -134,7 +129,7 @@ brw_codegen_vs_prog(struct brw_context *brw,
> /* vec4_visitor::setup_uniform_clipplane_values() also uploads user clip
> * planes as uniforms.
> */
> - param_count += c.key.base.nr_userclip_plane_consts * 4;
> + param_count += key->base.nr_userclip_plane_consts * 4;
>
> stage_prog_data->param =
> rzalloc_array(NULL, const gl_constant_value *, param_count);
> @@ -145,7 +140,7 @@ brw_codegen_vs_prog(struct brw_context *brw,
> GLbitfield64 outputs_written = vp->program.Base.OutputsWritten;
> prog_data.inputs_read = vp->program.Base.InputsRead;
>
> - if (c.key.copy_edgeflag) {
> + if (key->copy_edgeflag) {
> outputs_written |= BITFIELD64_BIT(VARYING_SLOT_EDGE);
> prog_data.inputs_read |= VERT_BIT_EDGEFLAG;
> }
> @@ -158,7 +153,7 @@ brw_codegen_vs_prog(struct brw_context *brw,
> * coords, which would be a pain to handle.
> */
> for (i = 0; i < 8; i++) {
> - if (c.key.point_coord_replace & (1 << i))
> + if (key->point_coord_replace & (1 << i))
> outputs_written |= BITFIELD64_BIT(VARYING_SLOT_TEX0 + i);
> }
>
> @@ -173,7 +168,7 @@ brw_codegen_vs_prog(struct brw_context *brw,
> * distance varying slots whenever clipping is enabled, even if the vertex
> * shader doesn't write to gl_ClipDistance.
> */
> - if (c.key.base.userclip_active) {
> + if (key->base.userclip_active) {
> outputs_written |= BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST0);
> outputs_written |= BITFIELD64_BIT(VARYING_SLOT_CLIP_DIST1);
> }
> @@ -182,13 +177,14 @@ brw_codegen_vs_prog(struct brw_context *brw,
> &prog_data.base.vue_map, outputs_written);
>
> if (0) {
> - _mesa_fprint_program_opt(stderr, &c.vp->program.Base, PROG_PRINT_DEBUG,
> + _mesa_fprint_program_opt(stderr, &vp->program.Base, PROG_PRINT_DEBUG,
> true);
And may be fix the indentation in the following line.
> }
>
> /* Emit GEN4 code.
> */
> - program = brw_vs_emit(brw, prog, &c, &prog_data, mem_ctx, &program_size);
> + program = brw_vs_emit(brw, mem_ctx, key, &prog_data,
> + &vp->program, prog, &program_size);
> if (program == NULL) {
> ralloc_free(mem_ctx);
> return false;
> @@ -202,7 +198,7 @@ brw_codegen_vs_prog(struct brw_context *brw,
> }
>
> brw_upload_cache(&brw->cache, BRW_CACHE_VS_PROG,
> - &c.key, sizeof(c.key),
> + key, sizeof(struct brw_vs_prog_key),
> program, program_size,
> &prog_data, sizeof(prog_data),
> &brw->vs.base.prog_offset, &brw->vs.prog_data);
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.h
> b/src/mesa/drivers/dri/i965/brw_vs.h
> index 0481c44..89a85c3 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.h
> +++ b/src/mesa/drivers/dri/i965/brw_vs.h
> @@ -50,21 +50,16 @@
> #define BRW_ATTRIB_WA_SIGN 32 /* interpret as signed in shader */
> #define BRW_ATTRIB_WA_SCALE 64 /* interpret as scaled in shader */
>
> -struct brw_vs_compile {
> - struct brw_vs_prog_key key;
> -
> - struct brw_vertex_program *vp;
> -};
> -
> #ifdef __cplusplus
> extern "C" {
> #endif
>
> const unsigned *brw_vs_emit(struct brw_context *brw,
> - struct gl_shader_program *prog,
> - struct brw_vs_compile *c,
> - struct brw_vs_prog_data *prog_data,
> void *mem_ctx,
> + const struct brw_vs_prog_key *key,
> + struct brw_vs_prog_data *prog_data,
> + struct gl_vertex_program *vp,
> + struct gl_shader_program *shader_prog,
> unsigned *program_size);
> void brw_vs_debug_recompile(struct brw_context *brw,
> struct gl_shader_program *prog,
> --
> 2.4.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
