<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Wed, Jul 25, 2018 at 9:30 AM Dylan Baker <<a href="mailto:dylan@pnwbakers.com">dylan@pnwbakers.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Quoting Jason Ekstrand (2018-07-24 16:51:26) > On July 24, 2018 09:05:05 Dylan Baker <<a href="mailto:dylan@pnwbakers.com" target="_blank">dylan@pnwbakers.com</a>> 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: <a href="mailto:mesa-stable@lists.freedesktop.org" target="_blank">mesa-stable@lists.freedesktop.org</a> > >> Cc: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> > >> --- > >> 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 > >> <a href="mailto:mesa-stable@lists.freedesktop.org" target="_blank">mesa-stable@lists.freedesktop.org</a> > >> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-stable" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-stable</a> > > > > 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; </blockquote><div> </div><div>Looks good. I couldn't remember of the old ones had a _var or not and was on my phone. </div></div></div>