[Mesa-dev] [PATCH 10/17] i965/vs: Move use_legacy_snorm_formula into the shader key
Jason Ekstrand
jason at jlekstrand.net
Fri Oct 9 06:02:47 PDT 2015
On Fri, Oct 9, 2015 at 5:58 AM, Pohjolainen, Topi
<topi.pohjolainen at intel.com> wrote:
> On Fri, Oct 09, 2015 at 05:53:43AM -0700, Jason Ekstrand wrote:
>> 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.
>
> I don't mind changing this. I was just concerned if we had a bug earlier as we
> didn't recompile when switching between gles3 and non-gles3.
We may yet have such a bug. When I kicked this series off to Jenkins
last night, it came back with GPU hangs on SNB and HSW.
Unfortunately, I have yet to be able to reproduce on my HSW so I'm not
sure my patches are actually to blame. If I can prove that it comes
from this series, this patch is definitely the most suspect.
--Jason
>>
>> >> ---
>> >> 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
More information about the mesa-dev
mailing list