[Mesa-dev] [PATCH 5/7] i965: Apply VS attribute workarounds in NIR.
Kenneth Graunke
kenneth at whitecape.org
Thu Jan 14 11:02:32 PST 2016
On Thursday, January 14, 2016 10:23:45 AM PST Matt Turner wrote:
> On Wed, Jan 13, 2016 at 8:33 PM, Kenneth Graunke <kenneth at whitecape.org>
wrote:
> > This patch re-implements the pre-Haswell VS attribute workarounds.
> > Instead of emitting shader code in the vec4 backend, we now simply
> > call a NIR pass to emit the necessary code.
> >
> > This simplifies the vec4 backend. Beyond deleting code, it removes
> > the primary use of ATTR as a destination. It also eliminates the
> > requirement that the vec4 VS backend express the ATTR file in terms
> > of VERT_ATTRIB_* locations, giving us a bit more flexibility.
> >
> > This approach is a little different: rather than munging the attributes
> > at the top, we emit code to fix them up when they're accessed. However,
> > we run the optimizer afterwards, so CSE should eliminate the redundant
> > math. It may even be able to fuse it with other calculations based on
> > the input value.
> >
> > shader-db does not handle non-default NOS settings, so I have no
> > statistics about this patch.
>
> Couldn't we hack it on (or off?) to get an idea?
>
> > Note that the scalar backend does not implement VS attribute
> > workarounds, as they are unnecessary on hardware which allows SIMD8 VS.
> >
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > ---
> >
> > src/mesa/drivers/dri/i965/Makefile.sources | 1 +
> > src/mesa/drivers/dri/i965/brw_nir.c | 19 ++-
> > src/mesa/drivers/dri/i965/brw_nir.h | 7 +-
> > .../dri/i965/brw_nir_attribute_workarounds.c | 178
> > +++++++++++++++++++++ src/mesa/drivers/dri/i965/brw_shader.cpp
> > | 2 +-
> > src/mesa/drivers/dri/i965/brw_vec4.cpp | 3 +
> > src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp | 2 +-
> > src/mesa/drivers/dri/i965/brw_vec4_vs_visitor.cpp | 109 -------------
> > 8 files changed, 204 insertions(+), 117 deletions(-)
> > create mode 100644
> > src/mesa/drivers/dri/i965/brw_nir_attribute_workarounds.c>
> > diff --git a/src/mesa/drivers/dri/i965/Makefile.sources
> > b/src/mesa/drivers/dri/i965/Makefile.sources index 5aeeca5..c654f94
> > 100644
> > --- a/src/mesa/drivers/dri/i965/Makefile.sources
> > +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> > @@ -42,6 +42,7 @@ i965_compiler_FILES = \
> >
> > brw_nir.h \
> > brw_nir.c \
> > brw_nir_analyze_boolean_resolves.c \
> >
> > + brw_nir_attribute_workarounds.c \
> >
> > brw_nir_opt_peephole_ffma.c \
> > brw_nir_uniforms.cpp \
> > brw_packed_float.c \
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_nir.c
> > b/src/mesa/drivers/dri/i965/brw_nir.c index 935529a..cdecc3d 100644
> > --- a/src/mesa/drivers/dri/i965/brw_nir.c
> > +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> > @@ -205,7 +205,9 @@ remap_patch_urb_offsets(nir_block *block, void
> > *closure)>
> > static void
> > brw_nir_lower_inputs(nir_shader *nir,
> >
> > const struct brw_device_info *devinfo,
> >
> > - bool is_scalar)
> > + bool is_scalar,
> > + bool use_legacy_snorm_formula,
> > + const uint8_t *vs_attrib_wa_flags)
> >
> > {
> >
> > switch (nir->stage) {
> >
> > case MESA_SHADER_VERTEX:
> > @@ -225,6 +227,9 @@ brw_nir_lower_inputs(nir_shader *nir,
> >
> > add_const_offset_to_base(nir, nir_var_shader_in);
> >
> > + brw_nir_apply_attribute_workarounds(nir, use_legacy_snorm_formula,
> > + vs_attrib_wa_flags);
> > +
> >
> > if (is_scalar) {
> >
> > /* Finally, translate VERT_ATTRIB_* values into the actual
> > registers.
> >
> > *
> >
> > @@ -497,12 +502,15 @@ brw_preprocess_nir(nir_shader *nir, bool is_scalar)
> >
> > nir_shader *
> > brw_nir_lower_io(nir_shader *nir,
> >
> > const struct brw_device_info *devinfo,
> >
> > - bool is_scalar)
> > + bool is_scalar,
> > + bool use_legacy_snorm_formula,
> > + const uint8_t *vs_attrib_wa_flags)
> >
> > {
> >
> > bool progress; /* Written by OPT and OPT_V */
> > (void)progress;
> >
> > - OPT_V(brw_nir_lower_inputs, devinfo, is_scalar);
> > + OPT_V(brw_nir_lower_inputs, devinfo, is_scalar,
> > + use_legacy_snorm_formula, vs_attrib_wa_flags);
> >
> > OPT_V(brw_nir_lower_outputs, devinfo, is_scalar);
> > OPT_V(nir_lower_io, nir_var_all, is_scalar ? type_size_scalar :
> > type_size_vec4);>
> > @@ -613,9 +621,10 @@ brw_create_nir(struct brw_context *brw,
> >
> > OPT_V(nir_lower_atomics, shader_prog);
> >
> > }
> >
> > - if (nir->stage != MESA_SHADER_TESS_CTRL &&
> > + if (nir->stage != MESA_SHADER_VERTEX &&
> > + nir->stage != MESA_SHADER_TESS_CTRL &&
> >
> > nir->stage != MESA_SHADER_TESS_EVAL) {
> >
> > - nir = brw_nir_lower_io(nir, devinfo, is_scalar);
> > + nir = brw_nir_lower_io(nir, devinfo, is_scalar, false, NULL);
> >
> > }
> >
> > return nir;
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_nir.h
> > b/src/mesa/drivers/dri/i965/brw_nir.h index 78b139b..5bfe40f 100644
> > --- a/src/mesa/drivers/dri/i965/brw_nir.h
> > +++ b/src/mesa/drivers/dri/i965/brw_nir.h
> > @@ -84,11 +84,16 @@ nir_shader *brw_create_nir(struct brw_context *brw,
> >
> > nir_shader *brw_preprocess_nir(nir_shader *nir, bool is_scalar);
> > nir_shader *brw_nir_lower_io(nir_shader *nir,
> >
> > const struct brw_device_info *devinfo,
> >
> > - bool is_scalar);
> > + bool is_scalar,
> > + bool use_legacy_snorm_formula,
> > + const uint8_t *vs_attrib_wa_flags);
> >
> > nir_shader *brw_postprocess_nir(nir_shader *nir,
> >
> > const struct brw_device_info *devinfo,
> > bool is_scalar);
> >
> > +bool brw_nir_apply_attribute_workarounds(nir_shader *nir,
> > + bool use_legacy_snorm_formula,
> > + const uint8_t *attrib_wa_flags);
> >
> > nir_shader *brw_nir_apply_sampler_key(nir_shader *nir,
> >
> > const struct brw_device_info
> > *devinfo,
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_nir_attribute_workarounds.c
> > b/src/mesa/drivers/dri/i965/brw_nir_attribute_workarounds.c new file mode
> > 100644
> > index 0000000..475fa1d
> > --- /dev/null
> > +++ b/src/mesa/drivers/dri/i965/brw_nir_attribute_workarounds.c
> > @@ -0,0 +1,178 @@
> > +/*
> > + * Copyright © 2016 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > a
> > + * copy of this software and associated documentation files (the
> > "Software"), + * to deal in the Software without restriction, including
> > without limitation + * the rights to use, copy, modify, merge, publish,
> > distribute, sublicense, + * and/or sell copies of the Software, and to
> > permit persons to whom the + * Software is furnished to do so, subject to
> > the following conditions: + *
> > + * The above copyright notice and this permission notice (including the
> > next + * paragraph) shall be included in all copies or substantial
> > portions of the + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND
> > NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT
HOLDERS
> > BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN
> > ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN
> > CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE
> > SOFTWARE.
> > + */
> > +
> > +#include "glsl/nir/nir_builder.h"
> > +#include "brw_nir.h"
> > +#include "brw_vs.h"
> > +
> > +/**
> > + * Prior to Haswell, the hardware can't natively support GL_FIXED or
> > + * 2_10_10_10_REV vertex formats. This pass inserts extra shader code
> > + * to produce the correct values.
> > + */
> > +
> > +struct attr_wa_state {
> > + nir_builder builder;
> > + bool impl_progress;
> > + bool use_legacy_snorm_formula;
> > + const uint8_t *wa_flags;
> > +};
> > +
> > +static bool
> > +apply_attr_wa_block(nir_block *block, void *void_state)
> > +{
> > + struct attr_wa_state *state = void_state;
> > + nir_builder *b = &state->builder;
> > +
> > + nir_foreach_instr_safe(block, instr) {
> > + if (instr->type != nir_instr_type_intrinsic)
> > + continue;
> > +
> > + nir_intrinsic_instr *intrin = nir_instr_as_intrinsic(instr);
> > + if (intrin->intrinsic != nir_intrinsic_load_input)
> > + continue;
> > +
> > + uint8_t wa_flags = state->wa_flags[intrin->const_index[0]];
> > + if (wa_flags == 0)
> > + continue;
> > +
> > + b->cursor = nir_after_instr(instr);
> > +
> > + nir_ssa_def *val = &intrin->dest.ssa;
> > +
> > + /* Do GL_FIXED rescaling for GLES2.0. Our GL_FIXED attributes
> > + * come in as floating point conversions of the integer values.
> > + */
> > + if (wa_flags & BRW_ATTRIB_WA_COMPONENT_MASK) {
> > + nir_ssa_def *comps[4];
> > + nir_ssa_def *factor = nir_imm_float(b, 1.0f / 65536.0f);
> > + for (int i = 0; i < val->num_components; i++) {
> > + if (i < (wa_flags & BRW_ATTRIB_WA_COMPONENT_MASK)) {
> > + comps[i] = nir_fmul(b, nir_channel(b, val, i), factor);
>
> It looks like the existing code uses wa_flags &
> BRW_ATTRIB_WA_COMPONENT_MASK to calculate a writemask and then does
> one multiply, while this will do potentially 3. Is that unavoidable?
>
Hmm, that's true. I could instead fmul the whole value, then use
nir_channel to pull out components from either the original or the
scaled value, then group them with a vec(). That would probably
be better (though my intuition isn't that great when it comes to
pulling apart and recombining values).
> The rest of the patch looks good.
>
> It would be nice to figure out a way to get shader-db to give us some
> numbers.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20160114/b2db2b96/attachment-0001.sig>
More information about the mesa-dev
mailing list