[Mesa-dev] [Mesa-stable] [PATCH] intel/compiler: Account for built-in uniforms in analyze_ubo_ranges

Dylan Baker dylan at pnwbakers.com
Tue Jul 24 16:04:24 UTC 2018


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?

Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180724/181b9ec3/attachment.sig>


More information about the mesa-dev mailing list