[Mesa-dev] [PATCH 03/11] i965: Pull stage_prog_data.nr_params out of the NIR shader
Jason Ekstrand
jason at jlekstrand.net
Thu Oct 1 07:58:09 PDT 2015
On Thu, Oct 1, 2015 at 7:52 AM, Iago Toral <itoral at igalia.com> wrote:
> On Wed, 2015-09-30 at 18:41 -0700, Jason Ekstrand wrote:
>> Previously, we had a bunch of code in each stage to figure out how many
>> slots we needed in stage_prog_data.param. This code was mostly identical
>> across the stages and had been copied and pasted around. Unfortunately,
>> this meant that any time you did something special, you had to add code for
>> it to each of these places. In particular, none of the stages took
>> subroutines into account; they were working entirely by accident. By
>> taking this data from the NIR shader, we know the exact number of entries
>> we need and everything goes a bit smoother.
>> ---
>> src/mesa/drivers/dri/i965/brw_cs.c | 4 ++--
>> src/mesa/drivers/dri/i965/brw_gs.c | 5 ++---
>> src/mesa/drivers/dri/i965/brw_vs.c | 16 ++++------------
>> src/mesa/drivers/dri/i965/brw_wm.c | 10 +++-------
>> 4 files changed, 11 insertions(+), 24 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_cs.c b/src/mesa/drivers/dri/i965/brw_cs.c
>> index 02eeeda..24120fb 100644
>> --- a/src/mesa/drivers/dri/i965/brw_cs.c
>> +++ b/src/mesa/drivers/dri/i965/brw_cs.c
>> @@ -30,6 +30,7 @@
>> #include "intel_mipmap_tree.h"
>> #include "brw_state.h"
>> #include "intel_batchbuffer.h"
>> +#include "glsl/nir/nir.h"
>>
>> static bool
>> brw_codegen_cs_prog(struct brw_context *brw,
>> @@ -55,8 +56,7 @@ brw_codegen_cs_prog(struct brw_context *brw,
>> * prog_data associated with the compiled program, and which will be freed
>> * by the state cache.
>> */
>> - int param_count = cs->base.num_uniform_components +
>> - cs->base.NumImages * BRW_IMAGE_PARAM_SIZE;
>> + int param_count = cp->program.Base.nir->num_uniforms;
>>
>> /* The backend also sometimes adds params for texture size. */
>> param_count += 2 * ctx->Const.Program[MESA_SHADER_COMPUTE].MaxTextureImageUnits;
>> diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c
>> index 61e7b2a..0cf7ec8 100644
>> --- a/src/mesa/drivers/dri/i965/brw_gs.c
>> +++ b/src/mesa/drivers/dri/i965/brw_gs.c
>> @@ -32,6 +32,7 @@
>> #include "brw_vec4_gs_visitor.h"
>> #include "brw_state.h"
>> #include "brw_ff_gs.h"
>> +#include "glsl/nir/nir.h"
>>
>>
>> bool
>> @@ -60,9 +61,7 @@ brw_codegen_gs_prog(struct brw_context *brw,
>> * every uniform is a float which gets padded to the size of a vec4.
>> */
>> struct gl_shader *gs = prog->_LinkedShaders[MESA_SHADER_GEOMETRY];
>> - int param_count = gs->num_uniform_components * 4;
>> -
>> - param_count += gs->NumImages * BRW_IMAGE_PARAM_SIZE;
>> + int param_count = gp->program.Base.nir->num_uniforms * 4;
>
> I think the vec4 nir backend does not handle image uniforms at the
> moment, does it? At least I see that the FS backend has code
> specifically for that in fs_visitor::nir_setup_uniform. Not sure if we
> support images in geometry stages though, but the code you remove seems
> to account for that...
We don't. When Curro initially implemented it, he did have vec4
support. However, that was dropped in favor of doing things
differently in the fs compiler. It may yet come back though. This
code was probably merged a bit speculatively but doesn't really hurt
anything.
>> c.prog_data.base.base.param =
>> rzalloc_array(NULL, const gl_constant_value *, param_count);
>> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
>> index e1a0d9c..391411c 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vs.c
>> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
>> @@ -109,18 +109,10 @@ brw_codegen_vs_prog(struct brw_context *brw,
>> * prog_data associated with the compiled program, and which will be freed
>> * by the state cache.
>> */
>> - int param_count;
>> - if (vs) {
>> - /* We add padding around uniform values below vec4 size, with the worst
>> - * case being a float value that gets blown up to a vec4, so be
>> - * conservative here.
>> - */
>> - param_count = vs->base.num_uniform_components * 4 +
>> - vs->base.NumImages * BRW_IMAGE_PARAM_SIZE;
>> - stage_prog_data->nr_image_params = vs->base.NumImages;
>> - } else {
>> - param_count = vp->program.Base.Parameters->NumParameters * 4;
>> - }
>> + int param_count = vp->program.Base.nir->num_uniforms;
>> + if (!brw->intelScreen->compiler->scalar_vs)
>> + param_count *= 4;
>
> Same thing here.
>
> Also, I guess this also means that for the scalar_vs cases we were
> computing a larger param_count than we really should, right?
They were, but it had nothing to do with images. It only had to do
with the fact that we were multiplying num_uniform_components by 4
which means that if your uniforms were all a bunch of floats, it would
be 4x the size needed.
>> /* vec4_visitor::setup_uniform_clipplane_values() also uploads user clip
>> * planes as uniforms.
>> */
>> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
>> index cc97d6a..08f2416 100644
>> --- a/src/mesa/drivers/dri/i965/brw_wm.c
>> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
>> @@ -35,6 +35,7 @@
>> #include "program/prog_parameter.h"
>> #include "program/program.h"
>> #include "intel_mipmap_tree.h"
>> +#include "glsl/nir/nir.h"
>>
>> #include "util/ralloc.h"
>>
>> @@ -173,14 +174,9 @@ brw_codegen_wm_prog(struct brw_context *brw,
>> * prog_data associated with the compiled program, and which will be freed
>> * by the state cache.
>> */
>> - int param_count;
>> - if (fs) {
>> - param_count = fs->base.num_uniform_components +
>> - fs->base.NumImages * BRW_IMAGE_PARAM_SIZE;
>> + int param_count = fp->program.Base.nir->num_uniforms;
>> + if (fs)
>> prog_data.base.nr_image_params = fs->base.NumImages;
>> - } else {
>> - param_count = fp->program.Base.Parameters->NumParameters * 4;
>> - }
>> /* The backend also sometimes adds params for texture size. */
>> param_count += 2 * ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxTextureImageUnits;
>> prog_data.base.param =
>
>
More information about the mesa-dev
mailing list