[Mesa-dev] [PATCH 5/7] i965: Apply VS attribute workarounds in NIR.
Matt Turner
mattst88 at gmail.com
Thu Jan 14 10:23:45 PST 2016
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?
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.
More information about the mesa-dev
mailing list