[Mesa-dev] [Mesa-stable] [PATCH] intel/compiler: Account for built-in uniforms in analyze_ubo_ranges
Jason Ekstrand
jason at jlekstrand.net
Wed Jul 25 16:38:15 UTC 2018
On Wed, Jul 25, 2018 at 9:30 AM Dylan Baker <dylan at pnwbakers.com> wrote:
> Quoting Jason Ekstrand (2018-07-24 16:51:26)
> > On July 24, 2018 09:05:05 Dylan Baker <dylan at pnwbakers.com> wrote:
> >
> > > Quoting Jason Ekstrand (2018-07-23 10:46:31)
> > >> The original pass only looked for load_uniform intrinsics but there
> are
> > >> a number of other places that could end up loading a push constant.
> One
> > >> obvious omission was images which always implicitly use a push
> constant.
> > >> Legacy VS clip planes also get pushed into the shader.
> > >>
> > >> Cc: mesa-stable at lists.freedesktop.org
> > >> Cc: Kenneth Graunke <kenneth at whitecape.org>
> > >> ---
> > >> src/intel/compiler/brw_nir.h | 1 +
> > >> .../compiler/brw_nir_analyze_ubo_ranges.c | 41 +++++++++++++++++--
> > >> src/intel/vulkan/anv_pipeline.c | 2 +-
> > >> src/mesa/drivers/dri/i965/brw_gs.c | 2 +-
> > >> src/mesa/drivers/dri/i965/brw_tcs.c | 2 +-
> > >> src/mesa/drivers/dri/i965/brw_tes.c | 2 +-
> > >> src/mesa/drivers/dri/i965/brw_vs.c | 2 +-
> > >> src/mesa/drivers/dri/i965/brw_wm.c | 2 +-
> > >> 8 files changed, 45 insertions(+), 9 deletions(-)
> > >>
> > >> diff --git a/src/intel/compiler/brw_nir.h
> b/src/intel/compiler/brw_nir.h
> > >> index 19442b47eae..7d82edafe46 100644
> > >> --- a/src/intel/compiler/brw_nir.h
> > >> +++ b/src/intel/compiler/brw_nir.h
> > >> @@ -148,6 +148,7 @@ void
> > >> brw_nir_lower_patch_vertices_in_to_uniform(nir_shader *nir);
> > >>
> > >> void brw_nir_analyze_ubo_ranges(const struct brw_compiler *compiler,
> > >> nir_shader *nir,
> > >> + const struct brw_vs_prog_key *vs_key,
> > >> struct brw_ubo_range out_ranges[4]);
> > >>
> > >> bool brw_nir_opt_peephole_ffma(nir_shader *shader);
> > >> diff --git a/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
> > >> b/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
> > >> index cd5137da06e..cfa531675fc 100644
> > >> --- a/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
> > >> +++ b/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
> > >> @@ -124,12 +124,29 @@ analyze_ubos_block(struct ubo_analysis_state
> *state,
> > >> nir_block *block)
> > >> continue;
> > >>
> > >> nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr);
> > >> - if (intrin->intrinsic == nir_intrinsic_load_uniform)
> > >> + switch (intrin->intrinsic) {
> > >> + case nir_intrinsic_load_uniform:
> > >> + case nir_intrinsic_image_deref_load:
> > >> + case nir_intrinsic_image_deref_store:
> > >> + case nir_intrinsic_image_deref_atomic_add:
> > >> + case nir_intrinsic_image_deref_atomic_min:
> > >> + case nir_intrinsic_image_deref_atomic_max:
> > >> + case nir_intrinsic_image_deref_atomic_and:
> > >> + case nir_intrinsic_image_deref_atomic_or:
> > >> + case nir_intrinsic_image_deref_atomic_xor:
> > >> + case nir_intrinsic_image_deref_atomic_exchange:
> > >> + case nir_intrinsic_image_deref_atomic_comp_swap:
> > >> + case nir_intrinsic_image_deref_size:
> > >> state->uses_regular_uniforms = true;
> > >> -
> > >> - if (intrin->intrinsic != nir_intrinsic_load_ubo)
> > >> continue;
> > >>
> > >> + case nir_intrinsic_load_ubo:
> > >> + break; /* Fall through to the analysis below */
> > >> +
> > >> + default:
> > >> + continue; /* Not a uniform or UBO intrinsic */
> > >> + }
> > >> +
> > >> nir_const_value *block_const = nir_src_as_const_value(intrin->src[0]);
> > >> nir_const_value *offset_const =
> nir_src_as_const_value(intrin->src[1]);
> > >>
> > >> @@ -179,6 +196,7 @@ print_ubo_entry(FILE *file,
> > >> void
> > >> brw_nir_analyze_ubo_ranges(const struct brw_compiler *compiler,
> > >> nir_shader *nir,
> > >> + const struct brw_vs_prog_key *vs_key,
> > >> struct brw_ubo_range out_ranges[4])
> > >> {
> > >> const struct gen_device_info *devinfo = compiler->devinfo;
> > >> @@ -197,6 +215,23 @@ brw_nir_analyze_ubo_ranges(const struct
> brw_compiler
> > >> *compiler,
> > >> _mesa_hash_table_create(mem_ctx, NULL, _mesa_key_pointer_equal),
> > >> };
> > >>
> > >> + switch (nir->info.stage) {
> > >> + case MESA_SHADER_VERTEX:
> > >> + if (vs_key && vs_key->nr_userclip_plane_consts > 0)
> > >> + state.uses_regular_uniforms = true;
> > >> + break;
> > >> +
> > >> + case MESA_SHADER_COMPUTE:
> > >> + /* Compute shaders use push constants to get the subgroup ID
> so it's
> > >> + * best to just assume some system values are pushed.
> > >> + */
> > >> + state.uses_regular_uniforms = true;
> > >> + break;
> > >> +
> > >> + default:
> > >> + break;
> > >> + }
> > >> +
> > >> /* Walk the IR, recording how many times each UBO block/offset is
> used. */
> > >> nir_foreach_function(function, nir) {
> > >> if (function->impl) {
> > >> diff --git a/src/intel/vulkan/anv_pipeline.c
> b/src/intel/vulkan/anv_pipeline.c
> > >> index 211cee788b8..1e6bd12b87d 100644
> > >> --- a/src/intel/vulkan/anv_pipeline.c
> > >> +++ b/src/intel/vulkan/anv_pipeline.c
> > >> @@ -472,7 +472,7 @@ anv_pipeline_compile(struct anv_pipeline
> *pipeline,
> > >> anv_nir_apply_pipeline_layout(pipeline, layout, nir, prog_data, map);
> > >>
> > >> if (stage != MESA_SHADER_COMPUTE)
> > >> - brw_nir_analyze_ubo_ranges(compiler, nir,
> prog_data->ubo_ranges);
> > >> + brw_nir_analyze_ubo_ranges(compiler, nir, NULL,
> prog_data->ubo_ranges);
> > >>
> > >> assert(nir->num_uniforms == prog_data->nr_params * 4);
> > >>
> > >> diff --git a/src/mesa/drivers/dri/i965/brw_gs.c
> > >> b/src/mesa/drivers/dri/i965/brw_gs.c
> > >> index 9acb0337e20..7263f6351e9 100644
> > >> --- a/src/mesa/drivers/dri/i965/brw_gs.c
> > >> +++ b/src/mesa/drivers/dri/i965/brw_gs.c
> > >> @@ -94,7 +94,7 @@ brw_codegen_gs_prog(struct brw_context *brw,
> > >> brw_nir_setup_glsl_uniforms(mem_ctx, gp->program.nir, &gp->program,
> > >> &prog_data.base.base,
> > >> compiler->scalar_stage[MESA_SHADER_GEOMETRY]);
> > >> - brw_nir_analyze_ubo_ranges(compiler, gp->program.nir,
> > >> + brw_nir_analyze_ubo_ranges(compiler, gp->program.nir, NULL,
> > >> prog_data.base.base.ubo_ranges);
> > >>
> > >> uint64_t outputs_written = gp->program.nir->info.outputs_written;
> > >> diff --git a/src/mesa/drivers/dri/i965/brw_tcs.c
> > >> b/src/mesa/drivers/dri/i965/brw_tcs.c
> > >> index 3b4642033fe..53611144ff5 100644
> > >> --- a/src/mesa/drivers/dri/i965/brw_tcs.c
> > >> +++ b/src/mesa/drivers/dri/i965/brw_tcs.c
> > >> @@ -185,7 +185,7 @@ brw_codegen_tcs_prog(struct brw_context *brw,
> struct
> > >> brw_program *tcp,
> > >> brw_nir_setup_glsl_uniforms(mem_ctx, nir, &tcp->program,
> > >> &prog_data.base.base,
> > >>
> compiler->scalar_stage[MESA_SHADER_TESS_CTRL]);
> > >> - brw_nir_analyze_ubo_ranges(compiler, tcp->program.nir,
> > >> + brw_nir_analyze_ubo_ranges(compiler, tcp->program.nir, NULL,
> > >> prog_data.base.base.ubo_ranges);
> > >> } else {
> > >> /* Upload the Patch URB Header as the first two uniforms.
> > >> diff --git a/src/mesa/drivers/dri/i965/brw_tes.c
> > >> b/src/mesa/drivers/dri/i965/brw_tes.c
> > >> index 6f37dfabbf8..b3220a94741 100644
> > >> --- a/src/mesa/drivers/dri/i965/brw_tes.c
> > >> +++ b/src/mesa/drivers/dri/i965/brw_tes.c
> > >> @@ -85,7 +85,7 @@ brw_codegen_tes_prog(struct brw_context *brw,
> > >> brw_nir_setup_glsl_uniforms(mem_ctx, nir, &tep->program,
> > >> &prog_data.base.base,
> > >> compiler->scalar_stage[MESA_SHADER_TESS_EVAL]);
> > >> - brw_nir_analyze_ubo_ranges(compiler, tep->program.nir,
> > >> + brw_nir_analyze_ubo_ranges(compiler, tep->program.nir, NULL,
> > >> prog_data.base.base.ubo_ranges);
> > >>
> > >> int st_index = -1;
> > >> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c
> > >> b/src/mesa/drivers/dri/i965/brw_vs.c
> > >> index f518649e751..69c0046bbb9 100644
> > >> --- a/src/mesa/drivers/dri/i965/brw_vs.c
> > >> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
> > >> @@ -181,7 +181,7 @@ brw_codegen_vs_prog(struct brw_context *brw,
> > >> brw_nir_setup_glsl_uniforms(mem_ctx, vp->program.nir, &vp->program,
> > >> &prog_data.base.base,
> > >> compiler->scalar_stage[MESA_SHADER_VERTEX]);
> > >> - brw_nir_analyze_ubo_ranges(compiler, vp->program.nir,
> > >> + brw_nir_analyze_ubo_ranges(compiler, vp->program.nir, key,
> > >> prog_data.base.base.ubo_ranges);
> > >> } else {
> > >> brw_nir_setup_arb_uniforms(mem_ctx, vp->program.nir, &vp->program,
> > >> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c
> > >> b/src/mesa/drivers/dri/i965/brw_wm.c
> > >> index c65ca166286..70fe3844442 100644
> > >> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> > >> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> > >> @@ -149,7 +149,7 @@ brw_codegen_wm_prog(struct brw_context *brw,
> > >> brw_nir_setup_glsl_uniforms(mem_ctx, fp->program.nir, &fp->program,
> > >> &prog_data.base, true);
> > >> brw_nir_analyze_ubo_ranges(brw->screen->compiler, fp->program.nir,
> > >> - prog_data.base.ubo_ranges);
> > >> + NULL, prog_data.base.ubo_ranges);
> > >> } else {
> > >> brw_nir_setup_arb_uniforms(mem_ctx, fp->program.nir, &fp->program,
> > >> &prog_data.base);
> > >> --
> > >> 2.17.1
> > >>
> > >> _______________________________________________
> > >> mesa-stable mailing list
> > >> mesa-stable at lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/mesa-stable
> > >
> > > Hi Jason,
> > >
> > > This applies, but won't build on the stable branch, because none of the
> > > intriniscs actually exist in 18.1. Do you want to try to pull in some
> > > additional
> > > patches to make this work, or should we drop this patch?
> >
> > Just get rid of the _deref in the image intrinsic names.
> >
> >
> >
>
> I had to apply this diff, does this look right:
>
> diff --git a/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
> b/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
> index da5ff4a3bbe..48bedabf31b 100644
> --- a/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
> +++ b/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
> @@ -126,17 +126,17 @@ analyze_ubos_block(struct ubo_analysis_state *state,
> nir_block *block)
> nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr);
> switch (intrin->intrinsic) {
> case nir_intrinsic_load_uniform:
> - case nir_intrinsic_image_deref_load:
> - case nir_intrinsic_image_deref_store:
> - case nir_intrinsic_image_deref_atomic_add:
> - case nir_intrinsic_image_deref_atomic_min:
> - case nir_intrinsic_image_deref_atomic_max:
> - case nir_intrinsic_image_deref_atomic_and:
> - case nir_intrinsic_image_deref_atomic_or:
> - case nir_intrinsic_image_deref_atomic_xor:
> - case nir_intrinsic_image_deref_atomic_exchange:
> - case nir_intrinsic_image_deref_atomic_comp_swap:
> - case nir_intrinsic_image_deref_size:
> + case nir_intrinsic_image_var_load:
> + case nir_intrinsic_image_var_store:
> + case nir_intrinsic_image_var_atomic_add:
> + case nir_intrinsic_image_var_atomic_min:
> + case nir_intrinsic_image_var_atomic_max:
> + case nir_intrinsic_image_var_atomic_and:
> + case nir_intrinsic_image_var_atomic_or:
> + case nir_intrinsic_image_var_atomic_xor:
> + case nir_intrinsic_image_var_atomic_exchange:
> + case nir_intrinsic_image_var_atomic_comp_swap:
> + case nir_intrinsic_image_var_size:
> state->uses_regular_uniforms = true;
> continue;
>
Looks good. I couldn't remember of the old ones had a _var or not and was
on my phone.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180725/101eb23d/attachment-0001.html>
More information about the mesa-dev
mailing list