[Mesa-dev] [PATCH 10/17] i965/vs: Move use_legacy_snorm_formula into the shader key

[Jason Ekstrand]

On Fri, Oct 9, 2015 at 12:00 AM, Pohjolainen, Topi
<topi.pohjolainen at intel.com> wrote:
> On Thu, Oct 08, 2015 at 05:22:42PM -0700, Jason Ekstrand wrote:
>> This is really an input into the shader compiler so it kind of makes sense
>> in the key.  Also, given where it's placed into the key, it doesn't
>> actually make it any bigger.
>
> But the key specifically controls recompiles, right? Before this didn't
> trigger a recompile but now it does. I'm probably missing something...

You're not missing anything.  It can cause recompiles.  However,
whether or not you're gles3 doesn't change that often. It can change
due to doing meta operations but those shaders are tiny so it's
probably not a big deal.

That said, this is one of the sketchier patches in the series.  I'm
100% ok with leaving it out and just passing use_legacy_snorm_formula
as a parameter.  I was just hoping I could reduce the parameter bloat
a little.

>> ---
>>  src/mesa/drivers/dri/i965/brw_program.h           | 2 ++
>>  src/mesa/drivers/dri/i965/brw_vec4.cpp            | 3 +--
>>  src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp | 9 ++++-----
>>  src/mesa/drivers/dri/i965/brw_vs.c                | 3 +++
>>  src/mesa/drivers/dri/i965/brw_vs.h                | 5 +----
>>  5 files changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_program.h b/src/mesa/drivers/dri/i965/brw_program.h
>> index cf0522a..2a9d1e9 100644
>> --- a/src/mesa/drivers/dri/i965/brw_program.h
>> +++ b/src/mesa/drivers/dri/i965/brw_program.h
>> @@ -91,6 +91,8 @@ struct brw_vs_prog_key {
>>
>>     bool clamp_vertex_color:1;
>>
>> +   bool use_legacy_snorm_formula:1;
>> +
>>     /**
>>      * How many user clipping planes are being uploaded to the vertex shader as
>>      * push constants.
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> index 4b8390f..4b03967 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> @@ -1992,8 +1992,7 @@ brw_vs_emit(struct brw_context *brw,
>>
>>        vec4_vs_visitor v(brw->intelScreen->compiler, brw, key, prog_data,
>>                          vp->Base.nir, brw_select_clip_planes(&brw->ctx),
>> -                        mem_ctx, shader_time_index,
>> -                        !_mesa_is_gles3(&brw->ctx));
>> +                        mem_ctx, shader_time_index);
>>        if (!v.run()) {
>>           if (prog) {
>>              prog->LinkStatus = false;
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
>> index 485a80e..9cf04cd 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp
>> @@ -77,7 +77,8 @@ vec4_vs_visitor::emit_prolog()
>>              /* ES 3.0 has different rules for converting signed normalized
>>               * fixed-point numbers than desktop GL.
>>               */
>> -            if ((wa_flags & BRW_ATTRIB_WA_SIGN) && !use_legacy_snorm_formula) {
>> +            if ((wa_flags & BRW_ATTRIB_WA_SIGN) &&
>> +                !key->use_legacy_snorm_formula) {
>>                 /* According to equation 2.2 of the ES 3.0 specification,
>>                  * signed normalization conversion is done by:
>>                  *
>> @@ -304,14 +305,12 @@ vec4_vs_visitor::vec4_vs_visitor(const struct brw_compiler *compiler,
>>                                   const nir_shader *shader,
>>                                   gl_clip_plane *clip_planes,
>>                                   void *mem_ctx,
>> -                                 int shader_time_index,
>> -                                 bool use_legacy_snorm_formula)
>> +                                 int shader_time_index)
>>     : vec4_visitor(compiler, log_data, &key->tex, &vs_prog_data->base, shader,
>>                    mem_ctx, false /* no_spills */, shader_time_index),
>>       key(key),
>>       vs_prog_data(vs_prog_data),
>> -     clip_planes(clip_planes),
>> -     use_legacy_snorm_formula(use_legacy_snorm_formula)
>> +     clip_planes(clip_planes)
>>  {
>>  }
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c
>> index 38de98f..ecaeefa 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vs.c
>> +++ b/src/mesa/drivers/dri/i965/brw_vs.c
>> @@ -31,6 +31,7 @@
>>
>>
>>  #include "main/compiler.h"
>> +#include "main/context.h"
>>  #include "brw_context.h"
>>  #include "brw_vs.h"
>>  #include "brw_util.h"
>> @@ -329,6 +330,8 @@ brw_vs_populate_key(struct brw_context *brw,
>>        key->clamp_vertex_color = ctx->Light._ClampVertexColor;
>>     }
>>
>> +   key->use_legacy_snorm_formula = !_mesa_is_gles3(&brw->ctx);
>> +
>>     /* _NEW_POINT */
>>     if (brw->gen < 6 && ctx->Point.PointSprite) {
>>        for (i = 0; i < 8; i++) {
>> diff --git a/src/mesa/drivers/dri/i965/brw_vs.h b/src/mesa/drivers/dri/i965/brw_vs.h
>> index c927cac..8cf9fa1 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vs.h
>> +++ b/src/mesa/drivers/dri/i965/brw_vs.h
>> @@ -91,8 +91,7 @@ public:
>>                     const nir_shader *shader,
>>                     gl_clip_plane *clip_planes,
>>                     void *mem_ctx,
>> -                   int shader_time_index,
>> -                   bool use_legacy_snorm_formula);
>> +                   int shader_time_index);
>>
>>  protected:
>>     virtual dst_reg *make_reg_for_system_value(int location,
>> @@ -113,8 +112,6 @@ private:
>>     struct brw_vs_prog_data * const vs_prog_data;
>>
>>     gl_clip_plane *clip_planes;
>> -
>> -   bool use_legacy_snorm_formula;
>>  };
>>
>>  } /* namespace brw */
>> --
>> 2.5.0.400.gff86faf
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev