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

Jason Ekstrand jason at jlekstrand.net
Tue Jul 24 23:51:26 UTC 2018


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.





More information about the mesa-dev mailing list