[Mesa-dev] [PATCH 03/11] panfrost/midgard: Flush undefineds to zero
Erik Faye-Lund
erik.faye-lund at collabora.com
Wed Jul 10 13:47:33 UTC 2019
On Wed, 2019-07-10 at 06:24 -0700, Alyssa Rosenzweig wrote:
> Fixes a buggy dEQP test.
>
> Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig at collabora.com>
> ---
> .../drivers/panfrost/ci/expected-failures.txt | 6 --
> src/gallium/drivers/panfrost/meson.build | 1 +
> .../drivers/panfrost/midgard/compiler.h | 6 ++
> .../panfrost/midgard/midgard_compile.c | 4 +
> .../panfrost/midgard/nir_undef_to_zero.c | 89
> +++++++++++++++++++
> 5 files changed, 100 insertions(+), 6 deletions(-)
> create mode 100644
> src/gallium/drivers/panfrost/midgard/nir_undef_to_zero.c
>
> diff --git a/src/gallium/drivers/panfrost/ci/expected-failures.txt
> b/src/gallium/drivers/panfrost/ci/expected-failures.txt
> index 33059118b49..6f52773cc73 100644
> --- a/src/gallium/drivers/panfrost/ci/expected-failures.txt
> +++ b/src/gallium/drivers/panfrost/ci/expected-failures.txt
> @@ -256,12 +256,6 @@ dEQP-
> GLES2.functional.rasterization.limits.points
> dEQP-GLES2.functional.shaders.builtin_variable.fragcoord_w
> dEQP-
> GLES2.functional.shaders.preprocessor.predefined_macros.line_2_fragme
> nt
> dEQP-
> GLES2.functional.shaders.preprocessor.predefined_macros.line_2_vertex
> -dEQP-GLES2.functional.shaders.random.all_features.fragment.0
> -dEQP-GLES2.functional.shaders.random.all_features.fragment.16
> -dEQP-GLES2.functional.shaders.random.all_features.fragment.5
> -dEQP-GLES2.functional.shaders.random.all_features.fragment.6
> -dEQP-GLES2.functional.shaders.random.all_features.vertex.0
> -dEQP-GLES2.functional.shaders.random.all_features.vertex.17
> dEQP-
> GLES2.functional.shaders.scoping.valid.local_variable_hides_function_
> parameter_fragment
> dEQP-
> GLES2.functional.shaders.scoping.valid.local_variable_hides_function_
> parameter_vertex
> dEQP-
> GLES2.functional.shaders.struct.local.dynamic_loop_assignment_fragmen
> t
> diff --git a/src/gallium/drivers/panfrost/meson.build
> b/src/gallium/drivers/panfrost/meson.build
> index 6b907f155ae..e1faa104f6b 100644
> --- a/src/gallium/drivers/panfrost/meson.build
> +++ b/src/gallium/drivers/panfrost/meson.build
> @@ -36,6 +36,7 @@ files_panfrost = files(
> 'midgard/midgard_liveness.c',
> 'midgard/midgard_ops.c',
>
> + 'midgard/nir_undef_to_zero.c',
> 'midgard/nir_lower_blend.c',
> 'midgard/nir_lower_framebuffer.c',
> 'midgard/cppwrap.cpp',
> diff --git a/src/gallium/drivers/panfrost/midgard/compiler.h
> b/src/gallium/drivers/panfrost/midgard/compiler.h
> index 5fe1d80692c..820831d35dd 100644
> --- a/src/gallium/drivers/panfrost/midgard/compiler.h
> +++ b/src/gallium/drivers/panfrost/midgard/compiler.h
> @@ -444,4 +444,10 @@ void emit_binary_bundle(
> midgard_bundle *bundle,
> struct util_dynarray *emission,
> int next_tag);
> +
> +/* NIR stuff */
> +
> +bool
> +nir_undef_to_zero(nir_shader *shader);
> +
> #endif
> diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
> b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
> index 1e08e349eee..9c86f19fd06 100644
> --- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c
> +++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c
> @@ -404,6 +404,8 @@ midgard_nir_lower_fdot2(nir_shader *shader)
> return progress;
> }
>
> +/* Flushes undefined values to zero */
> +
> static void
> optimise_nir(nir_shader *nir)
> {
> @@ -464,6 +466,8 @@ optimise_nir(nir_shader *nir)
> }
>
> NIR_PASS(progress, nir, nir_opt_undef);
> + NIR_PASS(progress, nir, nir_undef_to_zero);
> +
> NIR_PASS(progress, nir, nir_opt_loop_unroll,
> nir_var_shader_in |
> nir_var_shader_out |
> diff --git a/src/gallium/drivers/panfrost/midgard/nir_undef_to_zero.c
> b/src/gallium/drivers/panfrost/midgard/nir_undef_to_zero.c
> new file mode 100644
> index 00000000000..aacecc17e9d
> --- /dev/null
> +++ b/src/gallium/drivers/panfrost/midgard/nir_undef_to_zero.c
> @@ -0,0 +1,89 @@
> +/*
> + * Copyright (C) 2019 Collabora, Ltd.
> + *
> + * 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.
> + *
> + * Authors (Collabora):
> + * Alyssa Rosenzweig <alyssa.rosenzweig at collabora.com>
> + */
> +
> +/**
> + * @file
> + *
> + * Flushes undefined SSA values to a zero vector fo the appropriate
> component
> + * count, to avoid undefined behaviour in the resulting shader. Not
> required
> + * for conformance as use of uninitialized variables is explicitly
> left
> + * undefined by the spec. Works around buggy apps, however.
> + *
> + * Call immediately after nir_opt_undef. If called before, larger
> optimization
> + * opportunities from the former pass will be missed. If called
> outside of an
> + * optimization loop, constant propagation and algebraic
> optimizations won't be
> + * able to kick in to reduce stuff consuming the zero.
> + */
> +
> +#include "compiler/nir/nir.h"
> +#include "compiler/nir/nir_builder.h"
> +
> +bool nir_undef_to_zero(nir_shader *shader);
> +
> +bool
> +nir_undef_to_zero(nir_shader *shader)
> +{
> + bool progress = false;
> +
> + nir_foreach_function(function, shader) {
> + if (!function->impl) continue;
> +
> + nir_builder b;
> + nir_builder_init(&b, function->impl);
> +
> + nir_foreach_block(block, function->impl) {
> + nir_foreach_instr_safe(instr, block) {
> + if (instr->type !=
> nir_instr_type_ssa_undef) continue;
> +
> + nir_ssa_undef_instr *und =
> nir_instr_as_ssa_undef(instr);
> +
> + /* Get the required size */
> + unsigned c = und-
> >def.num_components;
> + unsigned s = und->def.bit_size;
> +
> + nir_const_value v[16];
> + assert(c <= 16);
> +
If you use NIR_MAX_VEC_COMPONENTS for the array-size instead, I think
you can drop the assert.
> + memset(v, 0, sizeof(v));
> +
> + b.cursor = nir_before_instr(instr);
> + nir_ssa_def *zero =
> nir_build_imm(&b, c, s, v);
> + nir_src zerosrc =
> nir_src_for_ssa(zero);
> +
> + nir_ssa_def_rewrite_uses(&und->def,
> zerosrc);
> +
> + progress |= true;
> + }
> + }
> +
> + nir_metadata_preserve(function->impl,
> nir_metadata_block_index | nir_metadata_dominance);
> +
> + }
> +
> + return progress;
> +}
> +
> +
More information about the mesa-dev
mailing list