[Mesa-dev] [PATCH 7/9] i965: Add a NIR analysis pass for determining when a boolean resolve is needed
Connor Abbott
cwabbott0 at gmail.com
Wed Mar 18 18:07:45 PDT 2015
Overall, I think it might be better to do this as a SSA pass right
before lowering to source modifiers. I don't think we would be running
any optimizations after it that depend on the output being all 0's or
all 1's, but if you're concerned about that then we could add separate
gen5-compare compare opcodes and lower the normal ones to those plus
any necessary resolves.
On Tue, Mar 17, 2015 at 10:17 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
> ---
> src/mesa/drivers/dri/i965/Makefile.sources | 2 +
> src/mesa/drivers/dri/i965/brw_nir.h | 45 ++++
> .../dri/i965/brw_nir_analize_boolean_resolves.c | 228 +++++++++++++++++++++
> 3 files changed, 275 insertions(+)
> create mode 100644 src/mesa/drivers/dri/i965/brw_nir.h
> create mode 100644 src/mesa/drivers/dri/i965/brw_nir_analize_boolean_resolves.c
>
> diff --git a/src/mesa/drivers/dri/i965/Makefile.sources b/src/mesa/drivers/dri/i965/Makefile.sources
> index c69441b..05ebbe9 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.sources
> +++ b/src/mesa/drivers/dri/i965/Makefile.sources
> @@ -73,6 +73,8 @@ i965_FILES = \
> brw_meta_util.h \
> brw_misc_state.c \
> brw_multisample_state.h \
> + brw_nir.h \
> + brw_nir_analize_boolean_resolves.c \
I must say, this is a rather unfortunate typo/spelling mistake... I
always read it as anal-ize and then my inner 5 year old comes out and
I giggle. To be honest, I couldn't even get through the entire thing
because of this. You're not the first person to do it though.
> brw_object_purgeable.c \
> brw_packed_float.c \
> brw_performance_monitor.c \
> diff --git a/src/mesa/drivers/dri/i965/brw_nir.h b/src/mesa/drivers/dri/i965/brw_nir.h
> new file mode 100644
> index 0000000..f79c5f1
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_nir.h
> @@ -0,0 +1,45 @@
> +/*
> + * Copyright © 2015 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.
> + */
> +
> +#pragma once
> +
> +#include "glsl/nir/nir.h"
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/* Flags set in the instr->pass_flags field by i965 analysis passes */
> +enum {
> + BRW_NIR_NON_BOOLEAN = 0x0,
> + BRW_NIR_BOOLEAN_NEEDS_RESOLVE = 0x1,
> + BRW_NIR_BOOLEAN_UNRESOLVED = 0x2,
> + BRW_NIR_BOOLEAN_NO_RESOLVE = 0x3,
> + BRW_NIR_BOOLEAN_MASK = 0x3,
> +};
> +
> +void brw_nir_analize_boolean_resolves(nir_shader *nir);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> diff --git a/src/mesa/drivers/dri/i965/brw_nir_analize_boolean_resolves.c b/src/mesa/drivers/dri/i965/brw_nir_analize_boolean_resolves.c
> new file mode 100644
> index 0000000..7d93471
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/brw_nir_analize_boolean_resolves.c
> @@ -0,0 +1,228 @@
> +/*
> + * Copyright © 2015 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.
> + *
> + * Authors:
> + * Jason Ekstrand <jason at jlekstrand.net>
> + */
> +
> +#include "brw_nir.h"
> +
> +/*
> + * This file implements an analysis pass that determines when we have to do
> + * a boolean resolve on GEN <= 5. Instructions that need a boolean resolve
> + * will have the booleans portion of the instr->pass_flags field set to
> + * BRW_NIR_BOOLEAN_NEEDS_RESOLVE.
> + */
> +
> +static uint8_t
> +get_resolve_state_for_src(nir_alu_instr *alu, unsigned src_idx)
> +{
> + nir_instr *src_instr;
> + if (alu->src[src_idx].src.is_ssa) {
> + src_instr = alu->src[src_idx].src.ssa->parent_instr;
> + } else {
> + src_instr = alu->src[src_idx].src.reg.reg->parent_instr;
> + }
> +
> + if (src_instr) {
> + uint8_t state = src_instr->pass_flags & BRW_NIR_BOOLEAN_MASK;
> +
> + /* If the source instruction nees resolve then, from the perspective
> + * of the user, it's a true boolean.
> + */
> + if (state == BRW_NIR_BOOLEAN_NEEDS_RESOLVE)
> + state = BRW_NIR_BOOLEAN_NO_RESOLVE;
> + return state;
> + } else {
> + return BRW_NIR_NON_BOOLEAN;
> + }
> +}
> +
> +static bool
> +src_mark_needs_resolve(nir_src *src, void *void_state)
> +{
> + if (src->is_ssa)
> + return true;
> +
> + if (src->reg.reg->parent_instr == NULL)
> + return true;
> +
> + nir_instr *src_instr = src->reg.reg->parent_instr;
> +
> + uint8_t bool_flags = src_instr->pass_flags & BRW_NIR_BOOLEAN_MASK;
> + if (bool_flags == BRW_NIR_BOOLEAN_UNRESOLVED) {
> + src_instr->pass_flags &= ~BRW_NIR_BOOLEAN_MASK;
> + src_instr->pass_flags |= BRW_NIR_BOOLEAN_NEEDS_RESOLVE;
> + }
> +
> + return true;
> +}
> +
> +static bool
> +analize_boolean_resolves_block(nir_block *block, void *void_state)
> +{
> + nir_foreach_instr(block, instr) {
> + /* Clear the boolean state */
> + instr->pass_flags &= ~BRW_NIR_BOOLEAN_MASK;
> +
> + switch (instr->type) {
> + case nir_instr_type_alu:
> + /* For ALU instructions, we handle [un]resolved booleans below. */
Might it be better to put the code for handling ALU instructions here
instead? In effect we're only running it for ALU instructions since
the rest of the cases end in a continue, so why not make that more
obvious by just moving it up here under the ALU case?
> + break;
> +
> + case nir_instr_type_load_const: {
> + /* For load_const instructions, it's a boolean exactly when it holds
> + * one of the values NIR_TRUE or NIR_FALSE.
> + */
> + nir_load_const_instr *load = nir_instr_as_load_const(instr);
> + if (load->value.u[0] == NIR_TRUE || load->value.u[0] == NIR_FALSE) {
> + instr->pass_flags |= BRW_NIR_BOOLEAN_NO_RESOLVE;
> + } else {
> + instr->pass_flags |= BRW_NIR_NON_BOOLEAN;
> + }
> + continue;
> + }
> +
> + default:
> + /* Everything else is an unknown non-boolean value and needs to
> + * have all sources resolved.
> + */
> + instr->pass_flags |= BRW_NIR_NON_BOOLEAN;
> + nir_foreach_src(instr, src_mark_needs_resolve, NULL);
> + continue;
> + }
> +
> + uint8_t bool_status;
> + nir_alu_instr *alu = nir_instr_as_alu(instr);
> + switch (alu->op) {
> + case nir_op_flt:
> + case nir_op_ilt:
> + case nir_op_ult:
> + case nir_op_fge:
> + case nir_op_ige:
> + case nir_op_uge:
> + case nir_op_feq:
> + case nir_op_ieq:
> + case nir_op_fne:
> + case nir_op_ine:
> + case nir_op_f2b:
> + case nir_op_i2b:
> + bool_status = BRW_NIR_BOOLEAN_UNRESOLVED;
> +
> + /* Even though the destination is allowed to be left unresolved,
> + * we need to resolve all the sources of a compare.
> + */
> + nir_foreach_src(instr, src_mark_needs_resolve, NULL);
> + break;
> +
> + case nir_op_imov:
> + case nir_op_inot:
> + if (alu->dest.write_mask == 1) {
> + /* This is a single-source instruction. Just copy the resolution
> + * state from the source.
> + */
> + bool_status = get_resolve_state_for_src(alu, 0);
> + } else {
> + bool_status = BRW_NIR_NON_BOOLEAN;
> + }
> + break;
> +
> + case nir_op_iand:
> + case nir_op_ior:
> + case nir_op_ixor: {
> + assert(alu->dest.write_mask == 1);
> +
> + uint8_t src0_flags = get_resolve_state_for_src(alu, 0);
> + uint8_t src1_flags = get_resolve_state_for_src(alu, 1);
> +
> + if (src0_flags == src1_flags) {
> + bool_status = src0_flags;
> + } else if (src0_flags == BRW_NIR_NON_BOOLEAN ||
> + src1_flags == BRW_NIR_NON_BOOLEAN) {
> + bool_status = BRW_NIR_NON_BOOLEAN;
> + } else {
> + /* At this point one of them is a true boolean and one is a
> + * boolean that needs a resolve. We could either resolve the
> + * unresolved source or we could resolve here. If we resolve
> + * the unresolved source then we get two resolves for the price
> + * of one. Just set this one to BOOLEAN_NO_RESOLVE and we'll
> + * let the code below force a resolve on the unresolved source.
> + */
> + bool_status = BRW_NIR_BOOLEAN_NO_RESOLVE;
> + }
> + break;
> + }
> +
> + default:
> + bool_status = BRW_NIR_NON_BOOLEAN;
> + }
> +
> + /* If the destination is SSA-like, go ahead allow unresolved booleans.
> + * If the destination register doesn't have a well-defined parent_instr
> + * we need to resolve immediately.
> + */
> + if (alu->dest.dest.reg.reg->parent_instr == NULL &&
> + bool_status == BRW_NIR_BOOLEAN_UNRESOLVED) {
> + bool_status = BRW_NIR_BOOLEAN_NEEDS_RESOLVE;
> + }
> +
> + instr->pass_flags |= bool_status;
> +
> + /* Finally, resolve sources if it's needed */
> + switch (bool_status) {
> + case BRW_NIR_BOOLEAN_NEEDS_RESOLVE:
> + case BRW_NIR_BOOLEAN_UNRESOLVED:
> + /* This instruction is either unresolved or we're doing the
> + * resolve here; leave the sources alone.
> + */
> + break;
> +
> + case BRW_NIR_BOOLEAN_NO_RESOLVE:
> + case BRW_NIR_NON_BOOLEAN:
> + nir_foreach_src(instr, src_mark_needs_resolve, NULL);
> + break;
> +
> + default:
> + unreachable("Invalid boolean flag");
> + }
> + }
> +
> + nir_if *following_if = nir_block_get_following_if(block);
> + if (following_if)
> + src_mark_needs_resolve(&following_if->condition, NULL);
> +
> + return true;
> +}
> +
> +static void
> +analize_boolean_resolves_impl(nir_function_impl *impl)
> +{
> + nir_foreach_block(impl, analize_boolean_resolves_block, NULL);
> +}
> +
> +void
> +brw_nir_analize_boolean_resolves(nir_shader *shader)
> +{
> + nir_foreach_overload(shader, overload)
> + if (overload->impl)
> + analize_boolean_resolves_impl(overload->impl);
> +}
> --
> 2.3.2
>
> _______________________________________________
> 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