On Wed, Jan 13, 2016 at 8:33 PM, Kenneth Graunke <[email protected]> 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 <[email protected]> > --- > 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? 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. _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
